Giter Club home page Giter Club logo

Comments (9)

flaix avatar flaix commented on May 23, 2024 1

Oooh, Artur I didn't pay attention at first to what you did there with your myjq. Very smart, love it.

Tried the same thing which leads me to an explanation.

The normal case with a short enough body. Response cut for brevity.

$ curl -nsSig -H 'Accept: application/vnd.github.v3+json' -H 'Content-Type: application/json' --data-binary @body10 -X POST https://api.github.com/repos/fzs/gitblit/releases
HTTP/1.1 201 Created
Date: Wed, 22 Jan 2020 20:06:24 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 2573
Server: GitHub.com
Status: 201 Created
X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4981
X-RateLimit-Reset: 1579723794
...
Vary: Accept-Encoding
X-GitHub-Request-Id: 5DCD:3BE42:1D79C8F:238EEA4:5E28AB3F

{
  "url": "https://api.github.com/repos/fzs/gitblit/releases/23051499",
  "assets_url": "https://api.github.com/repos/fzs/gitblit/releases/23051499/assets",
...

Now a call to the API with a longer body. Response again cut for brevity.

$ curl -nsSig -H 'Accept: application/vnd.github.v3+json' -H 'Content-Type: application/json' --data-binary @body15 -X POST https://api.github.com/repos/fzs/gitblit/releases
HTTP/1.1 100 Continue

HTTP/1.1 201 Created
Date: Wed, 22 Jan 2020 20:05:02 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 2820
Server: GitHub.com
{
Status: 201 Created
X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4982
X-RateLimit-Reset: 1579723794
...
Vary: Accept-Encoding
X-GitHub-Request-Id: 5DDB:3EC99:2680B66:2E8538C:5E28AAED

{
  "url": "https://api.github.com/repos/fzs/gitblit/releases/23051438",
  "assets_url": "https://api.github.com/repos/fzs/gitblit/releases/23051438/assets",
...

In the second case there is the HTTP/1.1 100 Continue response with a blank line, which is what ok.sh triggers on, and treats the rest a the response body.

$ printf ':\ncat -\n' > myjq; chmod 755 myjq
$ OK_SH_JQ_BIN=./myjq .github/ok.sh  create_release fzs gitblit tag_name=v123 draft=true body="$(awk -f randtext -v lineno=15 -v linelen=80 /usr/share/dict/words)"
HTTP/1.1 201 Created
Date: Wed, 22 Jan 2020 19:55:58 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 2791
Server: GitHub.com
{
Status: 201 Created
X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4983
X-RateLimit-Reset: 1579723794
...
Vary: Accept-Encoding
X-GitHub-Request-Id: 5C9B:2BB14:24CCFD8:2C6B471:5E28A8CD

{
  "url": "https://api.github.com/repos/fzs/gitblit/releases/23051178",
  "assets_url": "https://api.github.com/repos/fzs/gitblit/releases/23051178/assets",
...

ok.sh will need a special case handling for the 100 Continue response.
(And for the weird stray { in the headers.)

from ok.sh.

flaix avatar flaix commented on May 23, 2024 1

Certainly an option. But I could also see benefit in that behaviour. For example, you upload a large asset but got the authentication wrong. I have no experience and have not tried it out, but it would be better if that lead to an error response immediately, instead of after having sent the 100MB binary.

Is it much more than checking in _response for the response code 100 and discard everything up to the next empty line, after which the actual response will follow? As ok.sh is not interested in the 100 , which it could consider part of the "request".

from ok.sh.

flaix avatar flaix commented on May 23, 2024 1

Just as a proof of concept I toyed around with _response reading over the "100 Continue" and skipping to the actual response. It's not beautiful, only covers this one case, but proves the solution.
Feel free to base something better and cleaner on it, if you like to.
efe5c10

from ok.sh.

whiteinge avatar whiteinge commented on May 23, 2024 1

I skimmed through the HTTP parser sources for Node and Golang and both special-case the 100 status code and avoid trying to process the request body so it looks to me like your fix is the best possible fix. Thanks again for the stellar work here.

from ok.sh.

flaix avatar flaix commented on May 23, 2024

Same problem, different command.

I have the same problem when running create_release with a body= beyond a certain length.

  • ok.sh Version: 0.5.1
  • curl 7.54.0 (x86_64-apple-darwin18.0) libcurl/7.54.0 LibreSSL/2.6.5 zlib/1.2.11 nghttp2/1.24.1
  • jq-1.6
  • MacOS Mojave

I was unable to find out where the wrong input to jq slips through.
But I observed that it is related to the response code. From a certain body size on, curl will sent a Expect: 100-continue header and the response code will be 100, not 200. That is when the error happens.

You can check this e.g. with generating a release with a body of increasing length. I did that with the following script, and with 13 lines, it works fine, with 14, jq did puke.

awk -v lineno=14 -v linelen=80 'BEGIN { i = 0 } { i++; words[i] = $0 } END { line1 = ""; line2 = ""; srand(); n = 0; w = ""; nw = 0; for (j = 0; j < lineno; j++) { while (n <= linelen) { w = words[int(rand() * i) + 1]; nw = length(w); line1 = line2; if (n > 0) { line2 = line2 " " w; n += 1 + nw } else { line2 = w; n += nw } }; print line1; line2 = w; n = nw } }' /usr/share/dict/words

ok-sh-vvv.log

from ok.sh.

whiteinge avatar whiteinge commented on May 23, 2024

How interesting. I have never encountered 100 Continue before now. That awk script reproduces it fantastically. Thank you both for the excellent investigation.

Suggestions very welcome if you see a potential fix, otherwise I'll plan to poke at this over weekend. Offhand, it looks like _response will require a bit of rework.

from ok.sh.

whiteinge avatar whiteinge commented on May 23, 2024

I initially got wrapped up in reading the HTTP spec, but there's also a bunch of (grumpy) conversations on the web about this behavior.

Clearly both curl and GitHub are doing the right thing by sending and responding to that header, however it's not buying ok.sh much. It seems we can short-circuit that behavior in curl by including the -H 'Expect:' flag.

I still want to research if there's a more robust or canonical way we should be parsing an HTTP response but avoiding might be a nice, quick fix.

from ok.sh.

ArturKlauser avatar ArturKlauser commented on May 23, 2024

It might actually work. I just tried the curl line for uploading a 30MB asset and also only get a single "100 Continue" line for that size.

from ok.sh.

flaix avatar flaix commented on May 23, 2024

Yeah, if I understand it correctly, there should be only one, as it is a stop gap between the client sending headers and then the body.
As this client is only dealing with Github, it could be enough.

Sent with GitHawk

from ok.sh.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.