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.

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

Submit Code *[as Fork]* generates Payload Too Large #793

Open
Tracked by #432
Martii opened this issue Nov 2, 2015 · 8 comments
Open
Tracked by #432

Submit Code *[as Fork]* generates Payload Too Large #793

Martii opened this issue Nov 2, 2015 · 8 comments
Labels
bug You've guessed it... this means a bug is reported. needs discussion Blah, blah, blah, wahh, wahh, wahh, etc.

Comments

@Martii
Copy link
Member

Martii commented Nov 2, 2015

Since we are Content-Type: application/x-www-form-urlencoded encoding a scripts source this generates a larger transfer content size.

For example a script of actual size 782,701 Bytes can get expanded to 1,087,143 Bytes which renders an express page (e.g. not ours) of:

Payload Too Large 
  1. We're not handling this failure gracefully... not sure that we can with body-parser :\
  2. Unable to fork scripts that are too large. (this is the main bug)
  3. Unable to change (too large of) scripts through the site editor... although upload a file works because it isn't urlencoded. (work-around)

Possibilities:

  • There is a verify property in body-parser that could be used in our code but we would need to up the limit significantly and check/process in the verify routine if the buffer to UTF-8 string was too big in correlation to the maximum_upload_script_size.
  • ... (something else?)
@Martii Martii added bug You've guessed it... this means a bug is reported. needs discussion Blah, blah, blah, wahh, wahh, wahh, etc. labels Nov 2, 2015
@Martii
Copy link
Member Author

Martii commented Nov 2, 2015

Additional issue found with server accepting ANSI files ... we still do accept them but we should be serving UTF-8 now in the stream with #794 .

@sizzlemctwizzle
Copy link
Member

I'll take a closer look later today and see if I can find a solution.

@Martii Martii changed the title Submit Code *(as Fork) generates Payload Too Large Submit Code *[as Fork]* generates Payload Too Large Nov 2, 2015
@sizzlemctwizzle
Copy link
Member

Okay, so I tried building my own multipart/form-data request with the source of the script pulled out of ACE and sending it to the server via AJAX, but I had no luck getting formidable to parse the request body correctly. Anyway, I stashed my failed changes in a branchwith this one commit.

Sorry Marti, that was the best idea (essentially handling a script source submission like a script upload) I could come up with today. Maybe I'll come up with something else later, or maybe you can find a way to make my idea work...I have very little experience with raw request data.

@Martii
Copy link
Member Author

Martii commented Nov 3, 2015

Thanks... I'll have a look at it in a bit. :)

I was thinking about trying formidable too but wasn't quite ready for that just yet... still ironing out our Unicode issues (which is loosely related to this with urlencoding e.g. percent encoding) from #200 ,a tiny step at a time, because it's still present in a small use case manor.

@sizzlemctwizzle
Copy link
Member

We could always use socket.io to stream the script source to the server. But that'd be overkill.

@Martii
Copy link
Member Author

Martii commented Nov 3, 2015

Plus we would be excluding any browser (versions included) that doesn't have socket support... not sure we are at that point yet either. Interesting idea though.

@sizzlemctwizzle
Copy link
Member

Just for future reference, I never tried capturing the POST I sent using
some browser tool. Perhaps that could be enlightening. I'm sure there's one
that can parse multipart/form-data and can tell you when your request is
property formatted.
On Nov 2, 2015 8:34 PM, "Marti Martz" notifications@github.com wrote:

Plus we would be excluding any browser (versions included) that doesn't
have socket support... not sure we are at that point yet either.
Interesting idea though.


Reply to this email directly or view it on GitHub
#793 (comment)
.

Martii referenced this issue Jul 26, 2018
* This removes all known current deprecated deps found in express 3
* *body-parser* offers some native limits for form bodies sent... so mirroring that to transfer size upload limit
* Using some package defaults to eliminate 3 new deprecation messages
* Update README.md to reflect

Since *morgan* is the *express* endorsed request logger this closes #394 ... reopen if we need another/different one or to configure a written log.
@Martii
Copy link
Member Author

Martii commented Feb 10, 2021

Btw assuming it's a site fork, which this issue is, one should be able to fork a script on OUJS using just the metadata blocks to get the required fork status then file uploading (for those that don't have a GH account and repo) the complete actual code. Been a while since I've been in this section of code and there are other upcoming fires to put out.


I'm also contemplating upping the size limit but #1548 applies since we don't have WiredTiger for MongoDB yet with a volume for that. This means OUJS needs more donations to keep running as it at least doubles the monthly cost. Not a good idea to do this during the pandemic though without adequate donations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug You've guessed it... this means a bug is reported. needs discussion Blah, blah, blah, wahh, wahh, wahh, etc.
Development

No branches or pull requests

2 participants