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

Unsafe use of protected member response.raw._original_response #1093

Closed
2 tasks done
IlyaSukhanov opened this issue Jun 18, 2021 · 3 comments · Fixed by #1094
Closed
2 tasks done

Unsafe use of protected member response.raw._original_response #1093

IlyaSukhanov opened this issue Jun 18, 2021 · 3 comments · Fixed by #1094
Labels
bug Something isn't working new Needs triage. Comments are welcome!

Comments

@IlyaSukhanov
Copy link
Contributor

Checklist

  • I've searched for similar issues.
  • I'm using the the latest version of HTTPie.

What are the steps to reproduce the problem?

  1. Create an TransportPlugin that does not make use of urlib3/http.client
  2. Install/register TransportPlugin
  3. call http http+myprotocol://example/

https://github.com/urllib3/urllib3/blob/2f033880938c453895e087b285d50f979acbd0ce/src/urllib3/response.py#L362

What is the expected result?

See response from http+myprotocol://example/

What happens instead?

http: error: AttributeError: 'NoneType' object has no attribute 'msg'

Debug output

Please re-run the command with --debug, then copy the entire command & output and paste both below:

> http http+lambda://example/ --debug
HTTPie 2.4.0
Requests 2.25.1
Pygments 2.9.0
Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110]
...
Linux 5.10.0-4-amd64

<Environment {'colors': 8,
 'config': {'default_options': []},
 'config_dir': PosixPath('...'),
 'devnull': <property object at 0x7fb51ba345e0>,
 'is_windows': False,
 'log_error': <function Environment.log_error at 0x7fb51ba31940>,
 'program_name': 'http',
 'stderr': <_io.TextIOWrapper name='<stderr>' mode='w' encoding='utf-8'>,
 'stderr_isatty': True,
 'stdin': <_io.TextIOWrapper name='<stdin>' mode='r' encoding='utf-8'>,
 'stdin_encoding': 'utf-8',
 'stdin_isatty': True,
 'stdout': <_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>,
 'stdout_encoding': 'utf-8',
 'stdout_isatty': True}>

>>> requests.request(**{'auth': None,
 'data': RequestJSONDataDict(),
 'headers': {'User-Agent': b'HTTPie/2.4.0'},
 'method': 'get',
 'params': <generator object MultiValueOrderedDict.items at 0x7fb51b922350>,
 'url': 'http+lambda://example/'})


http: error: AttributeError: 'NoneType' object has no attribute 'msg'

Traceback (most recent call last):
  File ".../bin/http", line 8, in <module>
    sys.exit(main())
  File ".../lib/python3.9/site-packages/httpie/__main__.py", line 11, in main
    exit_status = main()
  File ".../lib/python3.9/site-packages/httpie/core.py", line 70, in main
    exit_status = program(
  File ".../lib/python3.9/site-packages/httpie/core.py", line 169, in program
    for message in messages:
  File "..../lib/python3.9/site-packages/httpie/client.py", line 109, in collect_messages
    headers=response.raw._original_response.msg._headers
AttributeError: 'NoneType' object has no attribute 'msg'

Urllib3 makes no assurance that original_response will be set. Would it be sensible to at least to try to get as much data out of response object directly. Even if this was limited to scenario when response.raw._original_response is None rather than have HTTPie crash. I would be happy to attempt to provide a patch to this effect but was wondering if this would be too far out of the normal case that HTTPie would accept.

Without safer handling of response.raw._original_response it appears that TransportPlugin functionality would be quite limited as one would have to emulate internal behavior of both urlib3 and http.client instead of just the Requests Response object.

This concern seems to have been identified at least once before #929 (comment).

@IlyaSukhanov IlyaSukhanov added bug Something isn't working new Needs triage. Comments are welcome! labels Jun 18, 2021
@BoboTiG
Copy link
Contributor

BoboTiG commented Jun 18, 2021

Hello @IlyaSukhanov,

Thanks for the report. Do you mind sharing the plugin to ease testing on my side?

Renforcing the robustness of that part seems interesting, so if you could come with a patch that is not a hammer, we could consider merging it ;)

@IlyaSukhanov
Copy link
Contributor Author

IlyaSukhanov commented Jun 19, 2021

@BoboTiG

Appreciate the quick response. And thank you for providing such a neat tool!

I've provided a patch. Appreciate any feedback and happy to make any requisite changes.

While I'd be happy to share code of plugin I'm working on, it relies a lot of setup to be able to reproduce the issue. Instead I included a much simpler implementation which triggers same scenario in tests. Hope you find it useful.

Best,
--IAS

@BoboTiG
Copy link
Contributor

BoboTiG commented Jun 22, 2021

Just for my information, there was a similar issue on requests: psf/requests#1534.

@BoboTiG BoboTiG linked a pull request Jun 23, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working new Needs triage. Comments are welcome!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants