Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestFactory: simplified #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JanTvrdik
Copy link
Contributor

  • removes URL filters which are very rarely used and could be implemented in userland
  • removes $_SERVER['ORIG_PATH_INFO'] because I don't know any HTTP server used in real-world which would actually require this

@hrach
Copy link
Contributor

hrach commented Dec 17, 2014

+1 never used it.

@JanTvrdik
Copy link
Contributor Author

I also did not manage to find a server where $_SERVER['SCRIPT_NAME'] would be undefined.

@JanTvrdik
Copy link
Contributor Author

@dg Can I remove the URL filters? They are magical and people who know about them prefer to turn them off anyway – see http://forum.nette.org/cs/9903-ne-kanonizace-lomitek-v-url or https://twitter.com/JanTvrdik/status/542389794085404673 Or does it fall into the same category as binary mode?

@dg
Copy link
Member

dg commented Dec 20, 2014

ad $_SERVER['ORIG_PATH_INFO'] & $_SERVER['SCRIPT_NAME']: detection is magic. See this versus this. Currently it somehow works and it is better to not touch it ;-)

ad filters: their problem is that they do not work with canonicalization… Filters are not used because nobody knows about them, they are hard to set, but they are useful. For example, they fix bad url parsers (like this), so it would be better to figure out how to solve the canonicalization.

dg added a commit that referenced this pull request Dec 26, 2014
dg added a commit that referenced this pull request Dec 26, 2014
dg added a commit that referenced this pull request Dec 27, 2014
dg added a commit that referenced this pull request Dec 27, 2014
…PT_FILENAME (BC break)

see #31

Conflicts:
	src/Http/RequestFactory.php
@dg
Copy link
Member

dg commented Dec 27, 2014

Partially merged by 37971f0

@dg dg force-pushed the master branch 4 times, most recently from e6f0643 to d6d5cc2 Compare December 27, 2014 04:21
@JanTvrdik
Copy link
Contributor Author

ad filters: I'm aware of what they can do, I'm just saying that nobody is using them for that. Even your websites don't use them. Wouldn't it be better to handle those use-cases in routing instead of RequestFactory? Or not handle them at all?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants