Fix for issue #680 + another fix #722

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants

laazyj commented Aug 20, 2012

Commit b8e7f18 fixes a bug with usage of Path.GetExtension(..) which caused routes to be truncated unexpectedly if you included a filename extension.

Commit 03f92e1 is to fix an "enumeration was modified" exception in the AspNet Hosting (see issue #680). I'm not 100% sure why this was required but I suspect the problem might've arisen if the request was aborted before the response was sent. This fix has certainly worked in our environment and isn't very intrusive.

laazyj added some commits Aug 13, 2012

@laazyj laazyj Enumerate copies of Response properties
Try and fix InvalidOperationException occurring in this method (Issue
#680)
03f92e1
@laazyj laazyj Stripping too many characters from the request path
Path.GetExtension includes the period.
b8e7f18
Owner

thecodejunkie commented Aug 20, 2012

Could you send on PR per feature? Makes it a lot easier for to review and triage. Thanks!

laazyj closed this Aug 20, 2012

laazyj commented Aug 20, 2012

Hmmm.. I'd love to but can't work out how!

laazyj reopened this Aug 21, 2012

laazyj commented Aug 21, 2012

Sorry, I can't work out how to split these 2 commits into separate pull requests. They're pretty small changes...
If you can give me a tip about how to split them into separate PRs (short of re-forking, feature-branching, etc) then I'd happily do it.

Owner

thecodejunkie commented Sep 20, 2012

I believe commit b8e7f18 is no longer applicable because we removed this code for 0.12.1 as it was part of a bug. Could you explain which scenarios that triggered the error that commit 03f92e1 solves?

Owner

thecodejunkie commented Sep 22, 2012

@laazyj perhaps you can close the pull request and send a new one, form a new branch, with only the commit 03f92e1 fix? Also see previous comment by me. Make sure to perform it on a fresh pull from master to avoid rebasing issues. Thank you

@thecodejunkie thecodejunkie added a commit to thecodejunkie/Nancy that referenced this pull request Oct 8, 2012

@thecodejunkie thecodejunkie Replace fix for #722 (should be closed) dab5592
Owner

thecodejunkie commented Oct 9, 2012

Fixed in #779

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment