nosurf breaks MultipartReader() about nosurf HOT 12 CLOSED

@bryanjeal Now that manual token verification is available, do you think it serves as a decent workaround to use when parsing multipart forms manually?

from nosurf.

I'm kind of stuck on how to get the both sides happy here. :(

from nosurf.

I wondered if using MultipartReader() would cause problems because it is trying to stream in the data.

Would it be possible to expose verifyToken()? Then I can mark my route as exempt, and process the CSRF token outside of your middleware.

from nosurf.

I'm kind of stuck on how to get the both sides happy here. :(

What's the other side?

from nosurf.

@shurcooL if nosurf parses multipart forms, they cannot be parsed in alternative ways by the end-user. If nosurf doesn't, it will not be able to verify multipart requests.

from nosurf.

What about this. Make a copy of the Request Body, then parse it. But restore the Body before calling the success http.Handler. Similar to how httputil.DumpRequest does it (see its source code).

from nosurf.

Copying the entire request body could/can consume
a non-negligible amount of memory (and therefore garbage).
On Tue, 4 Aug 2015 at 5:20 pm Dmitri Shuralyov [email protected]
wrote:

What about this. Make a copy of the Request Body, then parse it. But
restore the Body before calling the success handler. Similar to how
httputil.DumpRequest https://godoc.org/net/http/httputil#DumpRequest
does it (see its source code
http://gotools.org/net/http/httputil#dump-go-L177-L193).

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

from nosurf.

@elithrar That is a good point, making a copies of the request body can be detrimental to performance.

I wonder if it can be optimized by copying a part of the body until the CSRF token is found (and suggesting making it the first POST parameter), for example. Or some other means.

On another note, unless something here changes, I think the current documentation can be improved. It says:

... acts like a middleware and therefore is compatible with basically any Go HTTP application.

• Supports any http.Handler (frameworks, your own handlers, etc.) and acts like one itself.

It should be noted that this middleware may call ParseMultipartForm and ParseForm, therefore populating request's Form, PostForm, and MultipartForm fields, but consuming the body in the process.

Therefore, it will not work with http.Handlers that expect the original request body to be there unmodified.

from nosurf.

That might work, but it's a pretty sharp edge: if the token is after the other form values (e.g. a large file upload) that's a ton of implicit copying, especially for existing users.

Like @justinas said, I'm not quite sure how we can address this (my own CSRF lib has the same problem) and make both sides happy. Perhaps an option (not a huge fan of providing more options) to just do the copy operation (copy -> restore body) when parsing the multipart form? At least that way package users have some control over it.

from nosurf.

I'm not quite sure how we can address this

What do you think about my suggestion in #27 (comment) to improve documentation?

from nosurf.

Agree with improving the docs. I also think providing an example "how to" for package users (i.e. how to do the copy themselves) is worthwhile though: it can be a real pain.

from nosurf.

@justinas That is exactly what I was hoping for.

Thank you very much.

Closing this issue.

from nosurf.