Skip to content

Handle POST's larger than 1024 bytes correctly. #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
etr merged 5 commits into from
Jul 10, 2016
Merged

Handle POST's larger than 1024 bytes correctly. #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85848b9
Handle POST's larger than 1024 bytes correctly.
wlandry Apr 18, 2016
369707b
Add a way to limit size of uploads
wlandry Apr 18, 2016
2bdbda9
Properly limit the maximum size of the content and any processed post…
wlandry Apr 18, 2016
d727939
Add a function to check whether the content of a request was too large.
wlandry Apr 19, 2016
1fe733f
Merge 'etr/master' into large_POST.
wlandry Apr 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/httpserver/create_webserver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class create_webserver
_max_threads(0),
_max_connections(0),
_memory_limit(0),
_content_size_limit(static_cast<size_t>(-1)),
_connection_timeout(DEFAULT_WS_TIMEOUT),
_per_IP_connection_limit(0),
_log_access(0x0),
Expand Down Expand Up @@ -93,6 +94,7 @@ class create_webserver
_max_threads(0),
_max_connections(0),
_memory_limit(0),
_content_size_limit(static_cast<size_t>(-1)),
_connection_timeout(DEFAULT_WS_TIMEOUT),
_per_IP_connection_limit(0),
_log_access(0x0),
Expand Down Expand Up @@ -147,6 +149,10 @@ class create_webserver
{
_memory_limit = memory_limit; return *this;
}
create_webserver& content_size_limit(size_t content_size_limit)
{
_content_size_limit = content_size_limit; return *this;
}
create_webserver& connection_timeout(int connection_timeout)
{
_connection_timeout = connection_timeout; return *this;
Expand Down Expand Up @@ -333,6 +339,7 @@ class create_webserver
int _max_threads;
int _max_connections;
int _memory_limit;
size_t _content_size_limit;
int _connection_timeout;
int _per_IP_connection_limit;
log_access_ptr _log_access;
Expand Down
45 changes: 32 additions & 13 deletions src/httpserver/http_request.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,18 @@ class http_request
{
result = this->content;
}
/**
* Method to check whether the size of the content reached or exceeded content_size_limit.
* @return boolean
**/
bool content_too_large() const
{
return content.size()>=content_size_limit;
}
/**
* Method used to get the content of the query string..
* @return the query string in string representation
**/
const std::string get_querystring() const
{
return this->querystring;
Expand Down Expand Up @@ -361,7 +373,7 @@ class http_request
* Default constructor of the class. It is a specific responsibility of apis to initialize this type of objects.
**/
http_request():
content("")
content(""), content_size_limit(static_cast<size_t>(-1))
{
}
/**
Expand All @@ -381,6 +393,7 @@ class http_request
args(b.args),
querystring(b.querystring),
content(b.content),
content_size_limit(b.content_size_limit),
version(b.version),
requestor(b.requestor),
underlying_connection(b.underlying_connection)
Expand All @@ -398,6 +411,7 @@ class http_request
std::map<std::string, std::string, http::arg_comparator> args;
std::string querystring;
std::string content;
size_t content_size_limit;
std::string version;
std::string requestor;

Expand Down Expand Up @@ -442,11 +456,7 @@ class http_request
**/
void set_arg(const std::string& key, const std::string& value)
{
if (this->args[key].empty()) {
this->args[key] = value;
} else {
this->args[key].append(value);
}
this->args[key] = value.substr(0,content_size_limit);
Copy link
Owner

@etr etr Apr 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, it feels like throwing an exception would be more transparent - at least the library consumer will be notified that it happened and will avoid using partial data as argument unless they want to.

Copy link
Contributor Author

@wlandry wlandry Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did think of that. However, if I threw an exception, the functions webserver::build_request_args, and webserver::post_iterator would have to catch it immediately. They are invoked by libmicrohttpd, so they can not propagate exceptions. Then we would still need a way to indicate that too many characters were POST'ed.

With the solution I gave, the user can check whether each of the args has size >= content_size_limit. We could even add a simple function to check overflow. A C++98 version would be something like.

bool overflow() const
{
    for (std::map<std::string, std::string, http::arg_comparator>::const_iterator arg=args.begin();
          arg!=args.end(); ++arg)
    {
        if (arg->second.size()>=content_size_limit)
          return true;
    }
    return false;
}

Let me know what you think.

Copy link
Owner

@etr etr Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

I think, that having this kind of semantic, a utility such as the one you are describing could be very useful.

Copy link
Contributor Author

@wlandry wlandry Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out to be enough to just check the size of content. I added a function content_too_large().

I also had to roll back pellucide's modification since it conflicted with checking the size.

}
/**
* Method used to set an argument value by key.
Expand All @@ -456,19 +466,24 @@ class http_request
**/
void set_arg(const char* key, const char* value, size_t size)
{
if (this->args[key].empty()) {
this->args[key] = std::string(value, size);
} else {
this->args[key].append(value, size);
}
this->args[key] = std::string(value,
std::min(size, content_size_limit));
}
/**
* Method used to set the content of the request
* @param content The content to set.
**/
void set_content(const std::string& content)
{
this->content = content;
this->content = content.substr(0,content_size_limit);
}
/**
* Method used to set the maximum size of the content
* @param content_size_limit The limit on the maximum size of the content and arg's.
**/
void set_content_size_limit(size_t content_size_limit)
{
this->content_size_limit = content_size_limit;
}
/**
* Method used to append content to the request preserving the previous inserted content
Expand All @@ -478,6 +493,10 @@ class http_request
void grow_content(const char* content, size_t size)
{
this->content.append(content, size);
if (this->content.size() > content_size_limit)
{
this->content.resize (content_size_limit);
}
}
/**
* Method used to set the path requested.
Expand Down Expand Up @@ -568,7 +587,7 @@ class http_request
{
std::map<std::string, std::string>::const_iterator it;
for(it = args.begin(); it != args.end(); ++it)
this->args[it->first] = it->second;
this->args[it->first] = it->second.substr(0,content_size_limit);
}
/**
* Method used to set the username of the request.
Expand Down
1 change: 1 addition & 0 deletions src/httpserver/webserver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ class webserver
const int max_threads;
const int max_connections;
const int memory_limit;
const size_t content_size_limit;
const int connection_timeout;
const int per_IP_connection_limit;
log_access_ptr log_access;
Expand Down
4 changes: 3 additions & 1 deletion src/webserver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ webserver::webserver(const create_webserver& params):
max_threads(params._max_threads),
max_connections(params._max_connections),
memory_limit(params._memory_limit),
content_size_limit(params._content_size_limit),
connection_timeout(params._connection_timeout),
per_IP_connection_limit(params._per_IP_connection_limit),
log_access(params._log_access),
Expand Down Expand Up @@ -594,7 +595,7 @@ int webserver::post_iterator (void *cls, enum MHD_ValueKind kind,
)
{
struct details::modded_request* mr = (struct details::modded_request*) cls;
mr->dhr->set_arg(key, data, size);
mr->dhr->set_arg(key, mr->dhr->get_arg(key) + std::string(data, size));
return MHD_YES;
}

Expand Down Expand Up @@ -654,6 +655,7 @@ int webserver::bodyfull_requests_answer_first_step(
{
mr->second = true;
mr->dhr = new http_request();
mr->dhr->set_content_size_limit(content_size_limit);
const char *encoding = MHD_lookup_connection_value (
connection,
MHD_HEADER_KIND,
Expand Down