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

[BUG] http.query doesn't return body when HTTP response code is not 200 #63557

Closed
3 of 9 tasks
ggiesen opened this issue Jan 26, 2023 · 2 comments
Closed
3 of 9 tasks
Assignees
Labels
Bug broken, incorrect, or confusing behavior Execution-Module

Comments

@ggiesen
Copy link
Contributor

ggiesen commented Jan 26, 2023

Description
The utility module http.query and the corresponding execution/runner modules don't return the response body when the HTTP error code is not 200.

Setup
Set up a generic web server. I'm using the bw serve Bitwarden Vault Management REST API server in this case. Make a query to a URL that will return an HTTP 400.

Please be as specific as possible and give set-up details.

  • on-prem machine
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD
  • classic packaging
  • onedir packaging
  • used bootstrap to install

Steps to Reproduce the behavior

salt-run http.query http://localhost:8087/object/totp/580969d5-0c3e-4629-99bf-64185b07b588
error:
    HTTP 400: Bad Request
status:
    400

Here's the equivalent curl output for comparison:

curl -i http://localhost:8087/object/totp/580969d5-0c3e-4629-99bf-64185b07b588
HTTP/1.1 400 Bad Request
Content-Type: application/json; charset=utf-8
Content-Length: 63
Date: Thu, 26 Jan 2023 07:29:34 GMT
Connection: keep-alive
Keep-Alive: timeout=5

{"success":false,"message":"No TOTP available for this login."}

Expected behavior
Return the response body (if it exists) in addition to status code and error:

salt-run http.query http://localhost:8087/object/totp/580969d5-0c3e-4629-99bf-64185b07b588
body:
    {"success":false,"message":"No TOTP available for this login."}
error:
    HTTP 400: Bad Request
status:
    400

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
              Salt: 3005.1

Dependency Versions:
              cffi: Not Installed
          cherrypy: Not Installed
          dateutil: 2.6.1
         docker-py: Not Installed
             gitdb: Not Installed
         gitpython: Not Installed
            Jinja2: 2.10.1
           libgit2: Not Installed
          M2Crypto: 0.35.2
              Mako: Not Installed
           msgpack: 0.6.2
      msgpack-pure: Not Installed
      mysql-python: Not Installed
         pycparser: Not Installed
          pycrypto: Not Installed
      pycryptodome: Not Installed
            pygit2: Not Installed
            Python: 3.6.8 (default, Nov  9 2022, 09:57:34)
      python-gnupg: Not Installed
            PyYAML: 3.12
             PyZMQ: 20.0.0
             smmap: Not Installed
           timelib: Not Installed
           Tornado: 4.5.3
               ZMQ: 4.3.4

Salt Extensions:
 saltext.bitwarden: 0.0.1b11

System Versions:
              dist: almalinux 8.7 Stone Smilodon
            locale: UTF-8
           machine: x86_64
           release: 4.18.0-425.3.1.el8.x86_64
            system: Linux
           version: AlmaLinux 8.7 Stone Smilodon

Additional context
Here's the relevant section of code:

salt/salt/utils/http.py

Lines 618 to 621 in b59884b

except salt.ext.tornado.httpclient.HTTPError as exc:
ret["status"] = exc.code
ret["error"] = str(exc)
return ret

If I insert the following line between lines 620 and 621:

            ret["body"] = exc.response.body

I can get the desired output (albeit without support for options like decode or decode_body), and tornado.httpclient does generate a response object when an HTTPError/HTTPClientError is thrown per this.

@ggiesen ggiesen added Bug broken, incorrect, or confusing behavior needs-triage labels Jan 26, 2023
@Ch3LL Ch3LL added this to the Sulfur v3006.2 milestone Jul 20, 2023
@Ch3LL Ch3LL self-assigned this Jul 20, 2023
@Ch3LL
Copy link
Contributor

Ch3LL commented Aug 15, 2023

Fix: #64992 if you don't mind giving it a try

@Ch3LL
Copy link
Contributor

Ch3LL commented Aug 16, 2023

closed by #64992

@Ch3LL Ch3LL closed this as completed Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module
Projects
None yet
Development

No branches or pull requests

3 participants