Merged
Conversation
StokesAsselborn
force-pushed
the
feature/PostHandler
branch
from
9b1eb1b to
c08e137
Compare
Collaborator
Author
|
This is a first working version + some tests. To be improved, cleaned up later |
iboukhss
reviewed
Dec 5, 2025
Owner
iboukhss
left a comment
iboukhss
left a comment
There was a problem hiding this comment.
LGTM
My only worry is on uploading binary data, I think it will fail? I didn't test it
Otherwise well done on this
Comment on lines
+85
to
+95
| if (bytes == -1) { | ||
| // error occured | ||
| eob_reached_ = true; | ||
| if (headers_.empty()) { | ||
| HttpResponse res; | ||
| res.code = HttpResponse::kStatusDiskFull; | ||
| res.inline_body = "<h1> 507 Disk Full </h1>"; // to display a message during testing | ||
| headers_ = res.to_string(); | ||
| bytes_written_ = content_length_; // to ensure needs_input returns false | ||
| } | ||
| return 0; |
Owner
There was a problem hiding this comment.
I don't think this is valid without checking errno but it's probably fine
Collaborator
Author
There was a problem hiding this comment.
I noticed the same especially as bytes is of size_t. I will create an issue.
| Handler* handler = new UploadHandler(upload_path, content.size()); | ||
| ASSERT_TRUE(handler->needs_input()); | ||
| delete (handler); | ||
| remove(upload_path.c_str()); |
Collaborator
Author
There was a problem hiding this comment.
Just testing that needs_input returns true after init.
tests/upload_handler_unittest.cpp
Outdated
| remove(upload_path.c_str()); | ||
| } | ||
|
|
||
| UTEST(UploadHandlerTest, return_201) |
Owner
There was a problem hiding this comment.
return_201 was not a clear name (for me)
same for return_401
Collaborator
Author
There was a problem hiding this comment.
Ok let's change as you propose
Collaborator
Author
|
Closes #45 |
iboukhss
force-pushed
the
feature/PostHandler
branch
from
281f01f to
4eeaa74
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First version of the UploadHandler
Closes #45