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

Disallow the use of $_REQUEST #2373

Closed
kkmuffme opened this issue Jan 18, 2019 · 2 comments
Closed

Disallow the use of $_REQUEST #2373

kkmuffme opened this issue Jan 18, 2019 · 2 comments

Comments

@kkmuffme
Copy link

Like in VIP, but this makes sense generally:
https://vip.wordpress.com/documentation/vip-go/code-review-blockers-warnings-notices/#using-_request

$_REQUEST should never be used because it is hard to track where the data is coming from (was it POST, or GET, or a cookie?), which makes reviewing the code more difficult. Additionally, it makes it easy to introduce sneaky and hard to find bugs, as any of the aforementioned locations can supply the data, which is hard to predict. Much better to be explicit and use either $_POST or $_GET instead.

@Morerice
Copy link
Contributor

Morerice commented Dec 4, 2019

@gsherwood Just to confirm, since this is the first issue I'm trying to tackle on this project:

  • Since the issue seems general, the new sniff should be located under the src/Standards/Generic directory right?
  • And since something similar already exists, which however is pretty old,(src/Standards/MySource/Sniffs/PHP/GetRequestDataSniff.php) is it ok if I re-use some of its functionality?

@gsherwood
Copy link
Member

  • Since the issue seems general, the new sniff should be located under the src/Standards/Generic directory right?

Yes. That would be the right place to put it.

  • And since something similar already exists, which however is pretty old,(src/Standards/MySource/Sniffs/PHP/GetRequestDataSniff.php) is it ok if I re-use some of its functionality?

Yes. That would be a good idea. That whole MySource standard will be removed in version 4, so if there is anything worth keeping in that sniff, please feel free to use it.

Morerice added a commit to Morerice/PHP_CodeSniffer that referenced this issue Dec 8, 2019
@gsherwood gsherwood added this to the 3.5.4 milestone Dec 8, 2019
Morerice added a commit to Morerice/PHP_CodeSniffer that referenced this issue Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants