Edgewall Software
Modify

Opened 6 years ago

Closed 5 years ago

#658 closed enhancement (fixed)

Add workaround for Python 2.6's HTTPBasicAuthHandler deficiencies

Reported by: hodgestar Owned by: hodgestar
Priority: blocker Milestone: 0.6
Component: Build slave Version: 0.6b3
Keywords: Cc:
Operating System:

Description

The fix for Python issue 3819 introduced an infinite recursion bug into Python 2.6.x that is triggered by attempting to connect using Basic authentication with a bad username and/or password. This class fixes the problem using the simple solution outlined in a comment on issue 8797.

Attachments (1)

http-basic-auth-2.6-fix.diff (2.9 KB) - added by hodgestar 6 years ago.
Work around for Python 2.6 HTTPBasicAuthHandler issues.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by hodgestar

Work around for Python 2.6 HTTPBasicAuthHandler issues.

comment:1 follow-up: Changed 6 years ago by osimons

Nice. Patch reads well, and certainly looks like good research.

I've been staring at the source though, and I'm not sure a general compat is the right place to put it. With current layout, all slave-connection code is in slave.py. I think it makes sense to keep this one there too.

However, if you want to get a compat module going, it would be better to add it as bitten.util.compat - and keep the shared code in a location that is naturally available to both master and slave. If not the patch would also need to add the new top-level file to MANIFEST-SLAVE.in.

Anyway, +1 on comitting this code for 0.6.

comment:2 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by hodgestar

Replying to osimons:

I've been staring at the source though, and I'm not sure a general compat is the right place to put it. With current layout, all slave-connection code is in slave.py. I think it makes sense to keep this one there too.

My feeling was that slave.py is getting rather long and sprouting too many helper class. Perhaps now is not the best time to start cleaning that up though.

However, if you want to get a compat module going, it would be better to add it as bitten.util.compat - and keep the shared code in a location that is naturally available to both master and slave. If not the patch would also need to add the new top-level file to MANIFEST-SLAVE.in.

Ah. I missed that. bitten.util.compat is a better than bitten.compat then.

Anyway, +1 on comitting this code for 0.6.

Cool. If I don't get feedback from the original reporter soon I'll arrange to do my testing of the behaviour before and after the patch.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 5 years ago by wormyourhonor@…

Replying to hodgestar:

Replying to osimons:

I've been staring at the source though, and I'm not sure a general compat is the right place to put it. With current layout, all slave-connection code is in slave.py. I think it makes sense to keep this one there too.

My feeling was that slave.py is getting rather long and sprouting too many helper class. Perhaps now is not the best time to start cleaning that up though.

However, if you want to get a compat module going, it would be better to add it as bitten.util.compat - and keep the shared code in a location that is naturally available to both master and slave. If not the patch would also need to add the new top-level file to MANIFEST-SLAVE.in.

Ah. I missed that. bitten.util.compat is a better than bitten.compat then.

Anyway, +1 on comitting this code for 0.6.

Cool. If I don't get feedback from the original reporter soon I'll arrange to do my testing of the behaviour before and after the patch.

I, the original reporter, have verified my test case, invalid credentials:

  • FAIL using bitten-0.6.x@r969 - max recursion depth problem
  • PASS using patched bitten-0.6.x@r969. I receive "HTTP Error 401: Authorization Required".

For S&Gs, an unauthorized user with valid credentials exits normally with or without the patch, "HTTP Error 403: Forbidden"

comment:4 in reply to: ↑ 3 Changed 5 years ago by hodgestar

  • Status changed from new to assigned

Replying to wormyourhonor@…:

I, the original reporter, have verified my test case, invalid credentials:

  • FAIL using bitten-0.6.x@r969 - max recursion depth problem
  • PASS using patched bitten-0.6.x@r969. I receive "HTTP Error 401: Authorization Required".

For S&Gs, an unauthorized user with valid credentials exits normally with or without the patch, "HTTP Error 403: Forbidden"

Woot. Thanks!

comment:5 Changed 5 years ago by hodgestar

  • Resolution set to fixed
  • Status changed from assigned to closed

Patch committed in r974 and backported to 0.6.x in r975. The only change from the attached patch was moving bitten.compat to bitten.util.compat as suggested by osimons.

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain hodgestar.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.