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

Doc: State that Request.ParseForm() does not parse JSON payloads #25598

Closed
danieljsummers opened this issue May 27, 2018 · 7 comments
Closed
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge

Comments

@danieljsummers
Copy link

danieljsummers commented May 27, 2018

What did you expect to see?

I spent a good deal of time trying to determine why my XHR-style "forms" (actually JSON) were coming in blank.

What did you see instead?

After much debugging, head-desking, and searching, I discovered that this doesn't work that way. I've learned, but other people who have used other server frameworks may make the same mistake I did. In fact, I discovered it on Stack Overflow through someone else having the same problem.

I realize that a change to ParseForm would need to go through the language change proposal process. Obviously, nothing needs to be added to the language to be able to parse JSON payloads. However, I believe that a short note in this comment would help people understand that they'll need to parse JSON bodies themselves.

I'd be happy to update the comment to that effect, but before I go through that effort, I wanted to ask the team if that sort of thing would be welcome. So - let me know. :) (and thank you for your time)

@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label May 27, 2018
@meirf
Copy link
Contributor

meirf commented May 28, 2018

The doc includes:

For other HTTP methods, or when the Content-Type is not application/x-www-form-urlencoded, the request Body is not read, and r.PostForm is initialized to a non-nil, empty value.

I assume your case is application/json.

Am I misunderstanding your case or maybe the doc does not spell out this lacking prominent enough for your liking?

@danieljsummers
Copy link
Author

You're right that it does say that; I guess the negative implications of that statement didn't register with me.

I also probably ran down a rabbit trail because the router I was using at the time (Vestigo) puts its parameters in the form. I actually changed routers, thinking that Vestigo was whacking my posted values. Then, the form was always empty! :) (As one would expect, but that was the point where I got really confused.)

When I found the solution on Stack Overflow, I thought "Oh - of course." But, seeing the activity on that question, I thought it might be nice to call out this implication explicitly.

@meirf
Copy link
Contributor

meirf commented May 28, 2018

We generally want to keeps docs as succinct as possible since verbose docs leads to people ignoring them completely. Currently the doc is concise and correct on this matter.

It is staggering how popular that SO question is - 150k views. So maybe including something short about json being an example of unsupported content type could be helpful.

@pciet
Copy link
Contributor

pciet commented May 28, 2018

Which kind of HTTP method is your request? (https://tools.ietf.org/html/rfc2616#section-5.1.1)

@danieljsummers
Copy link
Author

Mine was a POST, as was the one being discussed in the Stack Overflow thread.

@meirf
Copy link
Contributor

meirf commented May 28, 2018

@danieljsummers all things considered, I think you should close this issue and we should rely on current docs which seem ok.

@danieljsummers
Copy link
Author

via PR, or without action?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

4 participants