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

data seems to be discarded in GET requests #97

Open
jsyeo opened this issue Dec 21, 2021 · 1 comment
Open

data seems to be discarded in GET requests #97

jsyeo opened this issue Dec 21, 2021 · 1 comment

Comments

@jsyeo
Copy link

jsyeo commented Dec 21, 2021

It seems like the library is discarding the data field when making GET requests. I understand that this doesn't really conform to the older RFC2616 spec but some http clients can do it e.g. curl and python's requests.

How to reproduce

$ nc -l 4242
requests.get("http://localhost:4242", data = "lol")

What is expected

nc should be printing this:

GET / HTTP/1.1
Content-Type: application/json
User-Agent: requests-scala
Accept-Encoding: gzip, deflate
Accept: */*
Cache-Control: no-cache
Pragma: no-cache
Host: localhost:4242
Connection: keep-alive

lol

What actually happens

Instead nc prints the request without the body.

GET / HTTP/1.1
Content-Type: application/json
User-Agent: requests-scala
Accept-Encoding: gzip, deflate
Accept: */*
Cache-Control: no-cache
Pragma: no-cache
Host: localhost:4242
Connection: keep-alive

Fix?

I think this line here is the culprit but I'm not sure why GET wasn't included in the if condition in the first place. https://github.com/com-lihaoyi/requests-scala/blob/master/requests/src/requests/Requester.scala#L254

        if (verb.toUpperCase == "POST" || verb.toUpperCase == "PUT" || verb.toUpperCase == "PATCH" || verb.toUpperCase == "DELETE") {
        ...
        }

Maybe we could add GET to it?

@jsyeo
Copy link
Author

jsyeo commented Dec 21, 2021

If you're wondering if there are any real world APIs that reads the body of GET requests, yes there are: https://api.slack.com/messaging/retrieving#conversations.

GET https://slack.com/api/conversations.history
Authorization: Bearer xoxb-your-token
{
  channel: "CONVERSATION_ID_HERE"
}

😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant