Skip to content
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

Jump to bottom

[DISCUSS] Validate new document writes against max_http_request_size #1253

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[DISCUSS] Validate new document writes against max_http_request_size #1253

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6755eac
feat: validate new document writes against max_http_request_size
janl Mar 29, 2018
ee70b56
wip
janl Mar 29, 2018
0e06697
fix comment
janl Jul 13, 2018
616ec02
hardcode mock uuid
janl Jul 13, 2018
797c6d7
remove debug log
janl Jul 13, 2018
e6ae447
smarter uuid
janl Jul 13, 2018
dac755b
Merge branch 'master' into feat/max-req-size
janl Jul 13, 2018
244442c
Better total (body + attachments) size checking for documents
nickva Jul 13, 2018
e2f556d
Merge pull request #6 from cloudant/better-total-doc-size-checking
janl Jul 13, 2018
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
Prev Previous commit
Next Next commit
smarter uuid
  • Loading branch information
@janl
janl committed Jul 13, 2018
commit e6ae447368d1f8683846fffcbd6283cb11ac63a5
2 changes: 1 addition & 1 deletion src/couch/src/couch_doc.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ from_json_obj_validate(EJson, DbName) ->
% make sure it is < max_http_request_size
validate_total_document_size(#doc{id=DocId, body=Body, atts=Atts0}=Doc) ->
MaxReqSize = config:get_integer("httpd", "max_http_request_size", 4294967296), % 4 GB
Boundary = <<"d07e231b4fc27759fd822449377fcba7">>,
Boundary = <<"00000000000000000000000000000000">>,
Atts = lists:map(fun couch_att:to_tuple/1, Atts0),
{_, DocSum} = couch_httpd_multipart:length_multipart_stream(Boundary,
?JSON_ENCODE(Body), Atts),
Copy link
Contributor

Choose a reason for hiding this comment

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

When we re-encode, we use jiffy, but unless it is a request made by the replicator, the user probably didn't use erlang to encode the data, so we could get a different value. There is also some performance loss in say re-encoding larger document bodies back to json just to check their size.

There is no canonical json encoding and so no canonical encoded json size. Above we are calculating the encoded size using a conservative size estimate (giving the user a benefit of the doubt) and with better performance (https://github.com/nickva/couch_ebench). Maybe make a version of length calculation that takes sizes and then we'd pass the already computed couch_ejson_size:encoded_size(Doc#doc.body) and 32 for boundary size.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @nickva, I’ll give this a ponder!

Expand Down