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

Jump to bottom

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
Closed

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

michalrus opened this issue Apr 16, 2021 · 8 comments
Labels
cmdline tool

Comments

@michalrus
Copy link

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 commented Apr 16, 2021 •
edited

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)

@bagder
Copy link
Member

bagder commented Apr 16, 2021

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

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

bagder commented Apr 16, 2021

(sorry, wrong button)

@jay
Copy link
Member

jay commented Apr 16, 2021

Is the problem that the number is zero-prefixed?

seems so

bagder added a commit that referenced this issue Apr 16, 2021
@bagder
test972: verify the json output with jsonlint
f79fd65 
Make sure one of the azure jobs has jsonlint installed so that the test
runs there.

Ref: #6905
@bagder bagder mentioned this issue Apr 16, 2021
Merged
bagder pushed a commit that referenced this issue Apr 16, 2021
@jay @bagder
tool_writeout: fix the HTTP_CODE json output
ca31fe2 
Fixes #6905
@bagder
Copy link
Member

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.

@gvanem
Copy link
Contributor

gvanem commented Apr 17, 2021 •
edited

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

bagder added a commit that referenced this issue Apr 17, 2021
@bagder
test972: verify the json output with jsonlint
6fc805d 
Make sure one of the azure jobs has jsonlint installed so that the test
runs there.

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

@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!

@jay
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cmdline tool
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bagder @gvanem @jay @michalrus