Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#8198 closed (duplicate)

Append slash broken when SCRIPT_NAME is present

Reported by: brooks.travis@… Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords: middleware
Cc: brooks.travis@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Since updating to trunk (r8273), the "append slash to url" functionality of CommonMiddleware no longer works.

Attachments (1)

urlresolvers.diff (524 bytes) - added by yogi 8 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by Alex Gaynor

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: worksforme
Status: newclosed

Can you be more specific, I just can access URLs without the trailing slash and they redirect correctly.

Changed 8 years ago by yogi

Attachment: urlresolvers.diff added

comment:2 Changed 8 years ago by yogi

Component: UncategorizedCore framework
Has patch: set
milestone: 1.0
Needs tests: set
Resolution: worksforme
Status: closedreopened
Summary: Append slash functionality of CommonMiddleware broken in trunk.Append slash functionality of CommonMiddleware broken with SCRIPT_NAME is present

This problem is specific to cases where a SCRIPT_NAME exists. The resolver doesn't take it into account, but it's passed in as a part of request.path. I've attached a diff which I think resolves the issue, but I'm new at this so someone more knowledgeable than I should look at it. It basically replaces the default regex of r'^/' with whatever get_script_prefix returns. I did some brief testing, seems to fix the issue.

Changed the milestone to 1.0 as this breaks full WSGI compatibility

Yay my first patch!

comment:3 Changed 8 years ago by yogi

Summary: Append slash functionality of CommonMiddleware broken with SCRIPT_NAME is presentAppend slash broken when SCRIPT_NAME is present

comment:4 Changed 8 years ago by brooks.travis@…

Yogi, you rock! Thanks for the patch. Hopefully, this will get thrown into trunk soon!

comment:5 Changed 8 years ago by Malcolm Tredinnick

Triage Stage: UnreviewedReady for checkin

Should use "^%s" % ... instead of using + on strings (can be fixed at checkin time), but other than that this is fine.

comment:6 Changed 8 years ago by Malcolm Tredinnick

Needs tests: unset
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Actually, this patch isn't quite right now that I think about it a bit. The resolver created in the get_resolver() method is cached and the prefix is per-thread, so shouldn't be cached. Some more work is needed here.

comment:7 Changed 8 years ago by yogi

Thanks for the correction + updates. Out of curiosity, why is it that the prefix's are stored per-thread (if it's not appropriate to post here, please email me). Seems pretty straightforward to implement caching on a per-thread basis, but I'd like to understand the reasoning behind it before coding a patch.

comment:8 Changed 8 years ago by Malcolm Tredinnick

Resolution: duplicate
Status: reopenedclosed

Closing as a dupe of #8381, which is solving the issue in a better way.

comment:9 Changed 5 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top