CUTCODEDOWN
Minimalist Semantic Markup

Welcome Guest
Please Login or Register

If you have registered but not recieved your activation e-mail in a reasonable amount of time, or have issues with using the registration form, please use our Contact Form for assistance. Include both your username and the e-mail you tried to register with.

Author Topic: cleanPath Function  (Read 1265 times)

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
cleanPath Function
« on: 4 Dec 2020, 08:26:36 pm »
Jason, could you please explain your intent with this function.


Code: [Select]
function cleanPath($path) {
    return trim(str_replace(['\\', '%5C'], '/', $path), '/');
}
To save time, let's just assume I am never wrong.

Jason Knight

  • Administrator
  • Hero Member
  • *****
  • Posts: 844
  • Karma: +159/-1
    • CutCodeDown -- Minimalist Semantic Markup
Re: cleanPath Function
« Reply #1 on: 4 Dec 2020, 10:44:44 pm »
Windows uses the wrong slashes, and we want to turn any entity slashes into forward slashes. Just makes sure the path is "safe" and "normal" before we start dicking with it.
I'll fix every flaw, I'll break every law, I'll tear up the rulebook if that's what it takes. You will see, I will crush this cold machine.

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
Re: cleanPath Function
« Reply #2 on: 5 Dec 2020, 07:42:02 pm »
You are passing $_SERVER['REQUEST_URI'] to the function.

Please provide details on exactly how to see backslashes in the $_SERVER['REQUEST_URI'] that this function is supposed to solve.
To save time, let's just assume I am never wrong.

Jason Knight

  • Administrator
  • Hero Member
  • *****
  • Posts: 844
  • Karma: +159/-1
    • CutCodeDown -- Minimalist Semantic Markup
Re: cleanPath Function
« Reply #3 on: 6 Dec 2020, 10:08:50 am »
You are passing $_SERVER['REQUEST_URI'] to the function.

Please provide details on exactly how to see backslashes in the $_SERVER['REQUEST_URI'] that this function is supposed to solve.

In windows run this:

Code: [Select]
<?php echo $_SERVER['REQUEST_URI'];
and access it via the filesystem instead of a server. In cases where PHP is allowed to run directly off the local filesystem (a horrific idea) the slashes are windows FS instead of http correct. 100% hurr durrz, so that's a safety precation.

I imagine you know why we ditch the URIencodes.
I'll fix every flaw, I'll break every law, I'll tear up the rulebook if that's what it takes. You will see, I will crush this cold machine.

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
Re: cleanPath Function
« Reply #4 on: 6 Dec 2020, 12:29:00 pm »
Quote
In windows run this:

In Windows Server 2016 IIS I did exactly that before I posted. There are no backslashes (\). It works exactly as expected. with forward slashes (/).

Based on this, the function is pointless, as it does nothing whatsoever.

Quote
In cases where PHP is allowed to run directly off the local filesystem (a horrific idea)

It's not just horrific, it is wrong and shouldn't be done and further makes the function pointless.

Why in the world are you going to develop an app that lets people do things wrong. If you are going to code to account for every wrong thing that consumers of your app may do you will end up with a monstrosity of spaghetti code.

Based on the previous I am declaring the function "bloat" which should be removed. There is just no valid reason for it.

Quote
I imagine you know why we ditch the URIencodes.

I would like to hear what you have to say about this.



To save time, let's just assume I am never wrong.

Jason Knight

  • Administrator
  • Hero Member
  • *****
  • Posts: 844
  • Karma: +159/-1
    • CutCodeDown -- Minimalist Semantic Markup
Re: cleanPath Function
« Reply #5 on: 6 Dec 2020, 02:09:33 pm »
Why in the world are you going to develop an app that lets people do things wrong.
And that's the difference in attitude. It's not about "letting" people do things wrong, it's about putting some simple small protections in place for WHEN they do it wrong.

Because they will. ALWAYS.

It's like database queries/actions where people don't wrap try/catch. Something that's currently MISSING in the PDO wrapper that I'm likely to take care of soon. Part of the whole reason for the database wrapper is to provide those types of intercepts so "you don't have to".
« Last Edit: 6 Dec 2020, 02:16:03 pm by Jason Knight »
I'll fix every flaw, I'll break every law, I'll tear up the rulebook if that's what it takes. You will see, I will crush this cold machine.

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
Re: cleanPath Function
« Reply #6 on: 6 Dec 2020, 02:37:38 pm »
It's not about "letting" people do things wrong, it's about putting some simple small protections in place for WHEN they do it wrong.

Don't you think it is better for it to fail rather than blindly allowing them to do something they shouldn't be? How are they going to know they do things wrong if you hide it from them? Lol, try raising a kid with that kind of rationale.
To save time, let's just assume I am never wrong.

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
Re: cleanPath Function
« Reply #7 on: 11 Dec 2020, 12:25:09 am »
You are passing $_SERVER['REQUEST_URI'] to the function.

After studying the code further I found the confusion.

In one place you do in fact pass $_SERVER['REQUEST_URI'] to the cleanPath function in the set method in common.lib. cleanPath does nothing for the $_SERVER['REQUEST_URI'] on windows,....but........the function does fix a slash issue on windows when you pass in $_SERVER['SCRIPT_NAME'] which you do when you define SCRIPT_PATH which is used by ROOT_HTTP. In this case the function is needed and fixes a slash issue on windows when the app is run from the server root.
To save time, let's just assume I am never wrong.

Jason Knight

  • Administrator
  • Hero Member
  • *****
  • Posts: 844
  • Karma: +159/-1
    • CutCodeDown -- Minimalist Semantic Markup
Re: cleanPath Function
« Reply #8 on: 11 Dec 2020, 07:02:36 am »
You also missed that it also kills escaped slashes, something we REALLY don't want in there "just in case" given the number of hacks that have happened on other systems thanks to that one simple thing...

Though perhaps it should also reject the forward slash when escaped?
I'll fix every flaw, I'll break every law, I'll tear up the rulebook if that's what it takes. You will see, I will crush this cold machine.

benanamen

  • Full Member
  • ***
  • Posts: 160
  • Karma: +14/-0
Re: cleanPath Function
« Reply #9 on: 11 Dec 2020, 12:13:28 pm »
kills escaped slashes, something we REALLY don't want in there "just in case"

Can you show me a case example where there are escaped slashes that would need to be dealt with?
To save time, let's just assume I am never wrong.

 

SMF spam blocked by CleanTalk

Advertisement