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

Do not crash without www-authentication header #1251

Merged
merged 1 commit into from Apr 3, 2019

Conversation

@NotSpecial
Copy link
Contributor

@NotSpecial NotSpecial commented Apr 3, 2019

Currently, there is an issue with abort(401). Per default,the keyword argument www_authenticate is None for the UnauthorizedException, i.e. abort(401) (as described in the Werkzeug docs), and the resulting exception attribute is e.www-authenticate = (None,).

In endpoints.py#L173, this (None,) is added to the header stack, which crashes response processing in render.py#L164, as (None,) cannot be unpacked to header, value.

In our API, an automated basic auth response does not make sense, yet calling abort(401) (without the www_authenticate) is currently impossible with Eve.

In this PR, I have modified the error endpoint to only add the www_authenticate header if it is a proper header, value pair and not the (None,) default.

I'd love to add a test as well, but I don't quite know where in the test suite this would fit in best. Any pointers are appreciated, and I'll get to it.

@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Apr 3, 2019

@NotSpecial are you on Werkzeug 0.14.1 or 0.15.1?

Loading

@NotSpecial
Copy link
Contributor Author

@NotSpecial NotSpecial commented Apr 3, 2019

0.15.2 actually, but I don't think they changed abort 401 again after 0.15.1

Edit: Just tried it with 0.15.1, same issue.

Loading

@NotSpecial
Copy link
Contributor Author

@NotSpecial NotSpecial commented Apr 3, 2019

Update: added link to the Werkzeug docs, where the default argument www_authenticate=None is described)

Loading

@nicolaiarocci nicolaiarocci added this to the 0.8.2 milestone Apr 3, 2019
@nicolaiarocci nicolaiarocci merged commit 26be349 into pyeve:master Apr 3, 2019
1 check passed
Loading
nicolaiarocci added a commit that referenced this issue Apr 3, 2019
@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Apr 3, 2019

Thanks.

Loading

@NotSpecial
Copy link
Contributor Author

@NotSpecial NotSpecial commented Apr 3, 2019

Awesome, thanks for merging and the fast response!

Loading

@NotSpecial NotSpecial deleted the fix-abort-crash branch Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants