Edgewall Software

Ticket #658 (closed enhancement: fixed)

Opened 4 years ago

Last modified 3 years ago

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

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

Change History

Changed 4 years ago by hodgestar

Work around for Python 2.6 HTTPBasicAuthHandler issues.

follow-up: ↓ 2   Changed 4 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.

in reply to: ↑ 1 ; follow-up: ↓ 3   Changed 4 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.

in reply to: ↑ 2 ; follow-up: ↓ 4   Changed 4 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"

in reply to: ↑ 3   Changed 4 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!

  Changed 3 years ago by hodgestar

  • status changed from assigned to closed
  • resolution set to fixed

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/Change #658 (Add workaround for Python 2.6's HTTPBasicAuthHandler deficiencies)

Author


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


Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.