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

fix key error in dictionary #2146

Closed
wants to merge 1 commit into from
Closed

Conversation

macdabby
Copy link

@macdabby macdabby commented Jan 6, 2022

What type of PR?

(Feature, enhancement, bug-fix, documentation)

What does this PR do?

Related issue(s)

Prerequisites

Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

  • In case of feature or enhancement: documentation updated accordingly
  • Unless it's docs or a minor change: add changelog entry file.

@mergify
Copy link
Contributor

mergify bot commented Jan 6, 2022

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

bors bot added a commit that referenced this pull request Jan 6, 2022
@@ -31,7 +31,7 @@ def nginx_authentication():
for key, value in headers.items():
response.headers[key] = str(value)
is_valid_user = False
is_from_webmail = headers['Auth-Port'] in ['10143', '10025']
is_from_webmail = headers.get('Auth-Port') in ['10143', '10025']
Copy link
Author

@macdabby macdabby Jan 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to test this on my system, but it looks like this key is present and accessed on line #15, however on line 29 it is replaced with a dictionary from nginx.handle_authentication() which is not returning an Auth-Port key. An alternative solution would be to update that function to always return the auth port. I think that might be better, but I dont know if it was omitted for a specific reason.

@bors
Copy link
Contributor

bors bot commented Jan 6, 2022

try

Build failed:

@nextgens
Copy link
Contributor

nextgens commented Jan 7, 2022

Thank you for sending a PR for this; #2150 is the preferred approach for fixing it

@nextgens nextgens closed this Jan 7, 2022
bors bot added a commit that referenced this pull request Jan 7, 2022
2150: fix 2145: exceptions may be thrown when login is invalid or rate-limits exceeded r=mergify[bot] a=nextgens

## What type of PR?

bug-fix

## What does this PR do?

Exceptions may be thrown when login is invalid or rate-limits exceeded for those running very recent builds of 1.9

For some reason I haven't caught it while testing #2130... that's when it was introduced.

### Related issue(s)
- close #2145
- close #2146
- #2130



Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Script crashing on bad auth : KeyError: 'Auth-Port'
2 participants