Opened 7 years ago

Closed 7 years ago

Last modified 4 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 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

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

Changed 7 years ago by yogi

comment:2 Changed 7 years ago by yogi

  • Component changed from Uncategorized to Core framework
  • Has patch set
  • milestone set to 1.0
  • Needs tests set
  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Summary changed from Append slash functionality of CommonMiddleware broken in trunk. to 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 7 years ago by yogi

  • Summary changed from Append slash functionality of CommonMiddleware broken with SCRIPT_NAME is present to Append slash broken when SCRIPT_NAME is present

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

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

comment:5 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Ready 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 7 years ago by mtredinnick

  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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 7 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 7 years ago by mtredinnick

  • Resolution set to duplicate
  • Status changed from reopened to closed

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

comment:9 Changed 4 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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