pallets / werkzeug

The comprehensive WSGI web application library.
https://werkzeug.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
6.66k stars 1.73k forks source link

`str(request.headers)` is empty #2985

Closed dgtlmoon closed 3 weeks ago

dgtlmoon commented 3 weeks ago

Greetings from changedetection.io, thanks for your work!

Our tests started failing, turns out related (somehow) to Werkzeug 3.1.0

We have a test endpoint that is looking at the headers that were sent with a request

https://github.com/dgtlmoon/changedetection.io/blob/7029d10f8bd22ac2379fcfe70bf57b67ebee69fc/changedetectionio/tests/util.py#L231

for example

    # Where we POST to as a notification
    @live_server.app.route('/test_notification_endpoint', methods=['POST', 'GET'])
    def test_notification_endpoint():
        p = request.headers

but request.headers is always empty (by empty i mean contains \r\n), downgrading to 3.0.6 fixed it (from 3.1.0)

Tried python 3.10, 3.11, and 3.12

pytest                       7.4.4
pytest-flask                 1.3.0
Werkzeug                     3.0.6 (this version is OK)

Strangely, all other variables in request look perfectly OK, tcpdump shows the headers are being sent without problem, HTTP_ type vars in the request look OK too

thanks!

candh commented 3 weeks ago

for anyone passing by: this also messes up reqparse of flask_restful

davidism commented 3 weeks ago

I can't reproduce this issue with the information provided. The following echoes back all headers sent by the browser:

from flask import Flask, request

app = Flask(__name__)

@app.route("/")
def echo():
    return request.headers.to_wsgi_list()

Header code was refactored a bit when moving the type annotations in, but all tests continue to pass in both Werkzeug and Flask. If you can create a minimal reproducible example, I do want to trace what's happening.

dgtlmoon commented 3 weeks ago

Hmm as a stand alone flask application I can also not reproduce it, but I can reproduce it in my pytest endpoint

davidism commented 3 weeks ago

I think I might have a reproducible example, if I return str(request.headers), Headers.__str__ seems to be returning an empty string, so something happened. I might know what happened, I'll step through with a debugger for a bit.

davidism commented 3 weeks ago

Aha, I was too clever. Headers stores a list of headers internally, and I thought I'd save some function calls/iterator setup by iterating over that list directly in __str__. EnvironHeaders (what request.headers is) subclasses Headers but ignores the internal list and uses the environ dict instead. So my refactor accidentally made it always look at the empty internal list instead of calling __iter__. I'll fix that and have 3.1.1 soon.

magnus-lycka commented 3 weeks ago

Calls with app.test_client() always gives status 401 for us with 4.1.0. Works fine with 4.0.6. Using flask_restx. Tests running in RHEL 8 and Windows 10. Same thing on both platforms.

davidism commented 3 weeks ago

@magnus-lycka It doesn't sound like that's related to this. Can you please report a separate bug with a minimal reproducible example?

magnus-lycka commented 3 weeks ago

@magnus-lycka It doesn't sound like that's related to this. Can you please report a separate bug with a minimal reproducible example?

I think it's the same issue. If headers are gone, I assume our auth won't work. This is a pretty big old monolith, and I'm definitely not able to create something reproducible this week. It's 16:46 Friday... I just noticed that 93 unit tests suddenly failed, with assertion for all sorts of values of status suddenly all being 401. Not sure how that works. Pinning to 3.0.6. for now, and will let you know if a fix for this missing headers problem fixes my problem or not.

davidism commented 3 weeks ago

@magnus-lycka I understand now, yeah if you're sending custom auth headers maybe this is related. Headers aren't gone though, only str(request.headers) was affected. I'm about to release 3.1.1 then, open a new issue if it persists after that. I tried to play around with test_client() really quick, but Flask and Werkzeug's tests with it all still pass before this fix.

dgtlmoon commented 3 weeks ago

thank you so much!

magnus-lycka commented 3 weeks ago

Turns out my optimism was unfounded. My issue remains with 3.1.1. Sigh... I'll try to distill my issue and file a new issue.