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

7.76.1 outputs invalid JSON in --write-out %{json} (a few versions back it was still fine) #6905

Closed
michalrus opened this issue Apr 16, 2021 · 8 comments

Comments

@michalrus
Copy link

@michalrus michalrus commented Apr 16, 2021

I did this

$ curl --write-out '%{json}' https://google.com/ --output /dev/null --silent 

{"content_type":"text/html; charset=UTF-8","errormsg":null,"exitcode":0,"filename_effective":"/dev/null","ftp_entry_path":null,"http_code":301,"http_connect":000,"http_version":"2","local_ip":"10.0.2.100","local_port":45384,"method":"GET","num_connects":1,"num_headers":12,"num_redirects":0,"proxy_ssl_verify_result":0,"redirect_url":"https://www.google.com/","referer":null,"remote_ip":"142.250.185.206","remote_port":443,"response_code":301,"scheme":"HTTPS","size_download":220,"size_header":639,"size_request":72,"size_upload":0,"speed_download":1057,"speed_upload":0,"ssl_verify_result":0,"time_appconnect":0.133558,"time_connect":0.046523,"time_namelookup":0.005388,"time_pretransfer":0.133639,"time_redirect":0.000000,"time_starttransfer":0.206770,"time_total":0.208079,"url":"https://google.com/","url_effective":"https://google.com/","urlnum":0,"curl_version":"libcurl/7.76.1 OpenSSL/1.1.1j zlib/1.2.11 brotli/1.0.9 nghttp2/1.42.0"}

Notice this part:

"http_connect":000

This is not valid JSON, and it breaks different parsers:

Error: Parse error on line 8:
...1,	"http_connect": 000,	"http_version"
----------------------^
Expecting 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '[', got 'undefined'

Perhaps you could add a unit test to verify said JSON validity with some external tool?

I expected the following

Valid JSON.

curl/libcurl version

curl 7.76.1 (x86_64-alpine-linux-musl) libcurl/7.76.1 OpenSSL/1.1.1j zlib/1.2.11 brotli/1.0.9 nghttp2/1.42.0
Release-Date: 2021-04-14
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets

operating system

alpine:3.13.2 on:

Linux 1df3df7de41f 5.10.23 #1-NixOS SMP Thu Mar 11 13:17:30 UTC 2021 x86_64 Linux
@jay
Copy link
Member

@jay jay commented Apr 16, 2021

curl/src/tool_writeout.c

Lines 260 to 272 in 566b74a

if(valid) {
if(use_json)
fprintf(stream, "\"%s\":", wovar->name);
if(wovar->id == VAR_HTTP_CODE || wovar->id == VAR_HTTP_CODE_PROXY)
fprintf(stream, "%03ld", longinfo);
else
fprintf(stream, "%ld", longinfo);
}
else {
if(use_json)
fprintf(stream, "\"%s\":null", wovar->name);
}

I think I was trying to imitate the existing non-json --write-out for the http codes.

diff --git a/src/tool_writeout.c b/src/tool_writeout.c
index 5296778..ed7a341 100644
--- a/src/tool_writeout.c
+++ b/src/tool_writeout.c
@@ -259,12 +259,13 @@ static int writeLong(FILE *stream, const struct writeoutvar *wovar,
 
   if(valid) {
     if(use_json)
-      fprintf(stream, "\"%s\":", wovar->name);
-
-    if(wovar->id == VAR_HTTP_CODE || wovar->id == VAR_HTTP_CODE_PROXY)
-      fprintf(stream, "%03ld", longinfo);
-    else
-      fprintf(stream, "%ld", longinfo);
+      fprintf(stream, "\"%s\":%ld", wovar->name, longinfo);
+    else {
+      if(wovar->id == VAR_HTTP_CODE || wovar->id == VAR_HTTP_CODE_PROXY)
+        fprintf(stream, "%03ld", longinfo);
+      else
+        fprintf(stream, "%ld", longinfo);
+    }
   }
   else {
     if(use_json)

Loading

@bagder
Copy link
Member

@bagder bagder commented Apr 16, 2021

Is the problem that the number is zero-prefixed? BTW, 'jq' parses this fine...

Loading

@bagder bagder closed this Apr 16, 2021
@bagder bagder reopened this Apr 16, 2021
@bagder
Copy link
Member

@bagder bagder commented Apr 16, 2021

(sorry, wrong button)

Loading

@jay
Copy link
Member

@jay jay commented Apr 16, 2021

Is the problem that the number is zero-prefixed?

seems so

Loading

bagder added a commit that referenced this issue Apr 16, 2021
Make sure one of the azure jobs has jsonlint installed so that the test
runs there.

Ref: #6905
bagder added a commit that referenced this issue Apr 16, 2021
@bagder
Copy link
Member

@bagder bagder commented Apr 16, 2021

@jay the new test 972 in #6906 seems to work so I pushed your proposed commit from here in that PR as well, as it worked fine locally for me with the new test.

Loading

@gvanem
Copy link
Contributor

@gvanem gvanem commented Apr 17, 2021

Is the problem that the number is zero-prefixed?

seems so

Yes. Testing using jsondump from https://github.com/zserge/jsmn:

c:\> curl -k --write-out "%%{json}" https://google.com/ --output NUL -s | jsondump - --verify
{
  "content_type": "text/html; charset=UTF-8",
  "errormsg": null,
  "exitcode": 0,
  "filename_effective": "NUL",
  "ftp_entry_path": null,
  "http_code": 301,
  "http_connect": line 1, col 153: [code=8] unexpected char

Loading

bagder added a commit that referenced this issue Apr 17, 2021
Make sure one of the azure jobs has jsonlint installed so that the test
runs there.

Ref: #6905
@bagder bagder closed this in 2f78be5 Apr 17, 2021
@michalrus
Copy link
Author

@michalrus michalrus commented Apr 19, 2021

@bagder @jay I encountered the same issue with 2 more fields:

$ curl --write-out '%{json}' http://i-dont-exist.xxx
curl: (6) Could not resolve host: i-dont-exist.xxx

{"content_type":null,"errormsg":"Could not resolve host: i-dont-exist.xxx","exitcode":6,"filename_effective":null,"ftp_entry_path":null,"http_code":000,"http_connect":000,"http_version":"0","local_ip":"","local_port":0,"method":"GET","num_connects":0,"num_headers":0,"num_redirects":0,"proxy_ssl_verify_result":0,"redirect_url":null,"referer":null,"remote_ip":"","remote_port":0,"response_code":000,"scheme":null,"size_download":0,"size_header":0,"size_request":0,"size_upload":0,"speed_download":0,"speed_upload":0,"ssl_verify_result":0,"time_appconnect":0.000000,"time_connect":0.000000,"time_namelookup":0.000000,"time_pretransfer":0.000000,"time_redirect":0.000000,"time_starttransfer":0.000000,"time_total":0.004487,"url":"http://i-dont-exist.xxx","url_effective":"http://i-dont-exist.xxx/","urlnum":0,"curl_version":"libcurl/7.76.1 OpenSSL/1.1.1j zlib/1.2.11 brotli/1.0.9 nghttp2/1.42.0"}
  • "http_connect":000
  • "response_code":000
  • "http_code":000

Looking at the diff of the fix, I think they should be covered as well, but asking just to make sure.

Thank you for a quick fix!

Loading

@jay
Copy link
Member

@jay jay commented Apr 19, 2021

  • "http_connect":000
  • "response_code":000
  • "http_code":000

Looking at the diff of the fix, I think they should be covered as well, but asking just to make sure.

yes

Loading

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

Successfully merging a pull request may close this issue.

None yet
4 participants