Skip to content

feat: refactoring CgiHandler#110

Merged
StokesAsselborn merged 5 commits into
devfrom
fix/cgi-handler
Jan 18, 2026
Merged

feat: refactoring CgiHandler#110
StokesAsselborn merged 5 commits into
devfrom
fix/cgi-handler

Conversation

@StokesAsselborn

Copy link
Copy Markdown
Collaborator

Main changes :

  1. Refactoring of the CgiHandler
  2. Tweaking the client.cpp for the refresh_interest_list(), write_to_virtual_file() and read_from_virtual_file()

Make test = ok

Specific curls :

Test OK
curl -i -G "http://localhost:8080/cgi-py/cgi_upper.py"   --data-urlencode "hello=$(python3 - <<'PY'
print("a"*666)
PY





)"
Test NOK
curl -i -G "http://localhost:8080/cgi-py/cgi_upper.py"   --data-urlencode "hello=$(python3 - <<'PY'
print("a"*666666)
PY





)"
Test OK but only once. Rerunning the command yields a broken pipe as the epoll list is not properly managed
curl -i -X POST "http://localhost:8080/cgi-py/cgi_upper.py"   -H "Content-Type: text/plain"   --data "hello from post"

@StokesAsselborn

Copy link
Copy Markdown
Collaborator Author

I pushed a working version of the refactored CgiHandler.
I let you judge which changes in the client.cpp need to be considered.

@StokesAsselborn StokesAsselborn marked this pull request as ready for review January 15, 2026 23:41
@iboukhss

Copy link
Copy Markdown
Owner

OK so I merged #109 just we have a stable base to work on. This PR tho I cannot merge right now. It's too hacky.

I will first push my fix for UploadHandler and then I can help you reconcile this.

We shouldn't have to change the client and server code that much to make things work.

The most problematic functions that are most likely buggy are Server::update_interest_list and Client::refresh_interest_list, too many if/else, everything else should be solid and simple to review.

@StokesAsselborn StokesAsselborn merged commit 2b0622a into dev Jan 18, 2026
1 check passed
@StokesAsselborn StokesAsselborn deleted the fix/cgi-handler branch January 18, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants