Comments (12)
Uploadify is a nasty mess of code. Yuck.
from formidable.
Hey bmoelk. Your fix does not work, because it assumes that you can do a lookahead on a data stream which you can't. Of course the odds will make your solution appear right most of the time here, but it's not a proper fix.
That being said, you should really report a bug with uploadify, they are violating the spec.
If you don't want to go through that hassle, I would probably accept a patch for this that doesn't rely on lookahead.
from formidable.
The only assumption I see is if there are additional buffers and the data after the LAST_BOUNDARY marker begins with two hyphens, this code will end the stream prematurely. It's not very likely, but I agree it is possible. Is that the assumption you're talking about?
I believe an option that would work is to pass through bytesReceived/bytesExpected to the multipart form parser and then check to see if the total bytes add up in addition to the check I've patched in above (double hyphens after the LAST_BOUNDARY).
I don't have a problem telling uploadify they are violating the spec, but I'm sure they aren't the only ones who will be. The other multi-part form parsers that we've used, do not choke on this so IMO it's not favorable to formidable to take a "we follow the specs" hardline. We just want stuff to work! :)
from formidable.
I'm definitely willing to support broken clients, it's the real world. This is just the first broken client that was reported and I'm short on time, that's why I'm not fixing it myself right away. If it was a browser, I'd fix this instantly.
I think the proper solution is to add another state called: QUIRKY_END
Then in the parsers end() function check if the state is QUIRKY_END || END.
Let me know if you can provide a patch like that, otherwise I'll do it myself at some point this or next week.
from formidable.
Ok, no worries or rush, my patch is "good enough" for me right now. I'm quite short on time myself, so it's not likely that I'll have time to work through a proper patch before the end of this month.
I appreciate your attention and responsiveness. Thanks!
from formidable.
I was about to post a bug on the uploadify forum, so I did some digging in regards to the spec (I think this is the right one):
http://www.ietf.org/rfc/rfc2388.txt
which references this spec:
http://www.ietf.org/rfc/rfc2046.txt
The relevant bits I think are on page 21:
multipart-body := [preamble CRLF]
dash-boundary transport-padding CRLF
body-part *encapsulation
close-delimiter transport-padding
[CRLF epilogue]
In this case, it appears the CRLF end is an optional element? Am I reading the spec correctly?
from formidable.
Just got bitten by this, this time from YUI Flash uploader. Trying to wrap my head around the parser code. Should I add the before-mentioned QUIRKY_END state given the fact that CRLF epilogue is optional? Any other guidance?
from formidable.
Yes, I think bmoelk is right about the spec. I just didn't have time to work on this yet.
If you want to look into it just make sure that all existing tests continue passing, and the new behavior of making the CRLF optional has a test as well.
from formidable.
I started with the failing test: ncr@7d5d8c9 If this path is worng, stop me before I put more time into this ;)
Update: the link is dead, I've recreated the fork.
from formidable.
Looks good! Unfortunately I don't have individual tests for the state transitions of the parser (like I do for node-mysql), so this is the best way to start this : ).
Also see my comment on that commit.
from formidable.
I've recreated my fork and just removed the "\r\n" from the existing test case as you suggested in the comments (now gone) under that "failing test" commit. I think I've managed to make the parser ignore everything after last boundary's two hyphens.
Here's the change: ncr@dd668b3
Should I create a separate pull request? Or more importantly - is this change any good? :)
from formidable.
Hi, the patch looks great! I applied it and pushed it out as part of v0.9.10:
Thank you so much for your help!
from formidable.
Related Issues (20)
- TS Options Type Definition missing createDirsFromUploads HOT 1
- Attach fields to req.body and files to req.files like multer HOT 1
- [deps] Check for breaking changes in Dependabot PRs HOT 1
- @cunha-ambisis you should create a new form on each request (inside app.use) HOT 1
- Since V3 I get "formidable is not a function" - using require not import HOT 12
- Make multipart parser less dependent on NodeJS HOT 2
- Regression in Formidable v3, which can crash a server HOT 7
- form.parse's Promise never resolves in tests HOT 8
- Hexoid Error (?)
- Buffer usage HOT 2
- Files parsed by `IncomingForm().parse` are not automatically cleaned up or deleted. HOT 6
- firstValues export in index HOT 4
- Build giving the error in `cloudflare-pages` preset HOT 2
- maxFileSize not working as it should HOT 1
- TypeScript - Unable to access FormidableError on formidable.errors HOT 9
- Convert 'firstValues' helper into an option for Formidable constructor. HOT 2
- Support the contentType of multipart field. HOT 2
- VERSION_NOTES.md is misleading
- Engines missing in `package.json` and wrong version of Node.js supported listed in README
- How to wait for the result of writeHandler in a formidable file upload request? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from formidable.