| Good code or bad code? [message #170181] |
Sun, 17 October 2010 04:20  |
MikeB
Messages: 65
Registered: September 2010
Karma: 0
|
Member |
|
|
I'm mostly playing with PHP to get a feeling for coding in it.
So as part of all this URI/URL/redirect stuff I spent some time looking
at the contents of $_SERVER and I came up with this code to find the
filename of the file I'm invoked from. Now one condition that I wanted
to cater for was if the filename had multiple "."s in the name, for
instance myfile.inc.php, or something like this.
So I came up with this piece of code.
$uriParts = explode("/",$_SERVER['REQUEST_URI']);
$thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
(strlen(end($uriParts))- strrpos(end($uriParts),'.')));
So I was wondering if that is good code or if I could have written it
better, since looking at it it is quite hard to understand.
Thanks
MikeB
I'll go away again for a while after this, I probably have been relying
on all y'alls good graces too much again.
|
|
|
|
| Re: Good code or bad code? [message #170185 is a reply to message #170181] |
Sun, 17 October 2010 06:33  |
Hamish Campbell
Messages: 15
Registered: September 2010
Karma: 0
|
Junior Member |
|
|
On Oct 17, 5:20 pm, MikeB <mpbr...@gmail.com> wrote:
> I'm mostly playing with PHP to get a feeling for coding in it.
>
> So as part of all this URI/URL/redirect stuff I spent some time looking
> at the contents of $_SERVER and I came up with this code to find the
> filename of the file I'm invoked from. Now one condition that I wanted
> to cater for was if the filename had multiple "."s in the name, for
> instance myfile.inc.php, or something like this.
>
> So I came up with this piece of code.
>
> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>
> So I was wondering if that is good code or if I could have written it
> better, since looking at it it is quite hard to understand.
Have a look at:
<http://nz.php.net/manual/en/function.pathinfo.php>
e.g:
$name = pathinfo($_SERVER['REQUEST_URI'], PATHINFO_FILENAME);
Regards
Hamish
|
|
|
|
| Re: Good code or bad code? [message #170187 is a reply to message #170181] |
Sun, 17 October 2010 12:47 |
Jerry Stuckle
Messages: 2598
Registered: September 2010
Karma: 0
|
Senior Member |
|
|
On 10/17/2010 12:20 AM, MikeB wrote:
> I'm mostly playing with PHP to get a feeling for coding in it.
>
> So as part of all this URI/URL/redirect stuff I spent some time looking
> at the contents of $_SERVER and I came up with this code to find the
> filename of the file I'm invoked from. Now one condition that I wanted
> to cater for was if the filename had multiple "."s in the name, for
> instance myfile.inc.php, or something like this.
>
> So I came up with this piece of code.
>
> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>
> So I was wondering if that is good code or if I could have written it
> better, since looking at it it is quite hard to understand.
>
> Thanks
> MikeB
>
> I'll go away again for a while after this, I probably have been relying
> on all y'alls good graces too much again.
In addition to what Hammish said, this information is sent by the
browser and cannot be trusted. Some browsers may not send it, and if it
is sent, it may be falsified (i.e. by a hacker).
--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex(at)attglobal(dot)net
==================
|
|
|
|
| Re: Good code or bad code? [message #170189 is a reply to message #170187] |
Sun, 17 October 2010 13:12 |
MikeB
Messages: 65
Registered: September 2010
Karma: 0
|
Member |
|
|
Jerry Stuckle wrote:
> On 10/17/2010 12:20 AM, MikeB wrote:
>> I'm mostly playing with PHP to get a feeling for coding in it.
>>
>> So as part of all this URI/URL/redirect stuff I spent some time looking
>> at the contents of $_SERVER and I came up with this code to find the
>> filename of the file I'm invoked from. Now one condition that I wanted
>> to cater for was if the filename had multiple "."s in the name, for
>> instance myfile.inc.php, or something like this.
>>
>> So I came up with this piece of code.
>>
>> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
>> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
>> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>>
>> So I was wondering if that is good code or if I could have written it
>> better, since looking at it it is quite hard to understand.
>>
>> Thanks
>> MikeB
>>
>> I'll go away again for a while after this, I probably have been relying
>> on all y'alls good graces too much again.
>
> In addition to what Hammish said, this information is sent by the
> browser and cannot be trusted. Some browsers may not send it, and if it
> is sent, it may be falsified (i.e. by a hacker).
>
I said I would shut up for a while, but now you bring up something else.
I wrote that code to find the fiilename (eg. index) so that I could
dynamically derive the name of an accompanying template file.
So if I'm running from index.php, I could derive index.tpl for a Smarty
template to accompany the php file.
If a hacker falsifies this, the template won't match the php file
creating the output and the page (s)he sees will be all messed up.
so that brings up two questions:
1. Is there a better way to dynamically derive a base filename? Eg. Is
there a php function that I can use to get the name of the executing
file? That may be better/safer then.
2. Is the risk of this being hacked sufficient that I should rather
statically code the template filename and then go through the hassle of
recoding the name every time I change (or move) the base file around?
I'm not sure that there is an risk to a website if the Smarty template
gets messed up, but I can see that there might be other uses that could
me more risky, so I'll certainly bear that in mind.
|
|
|
|
| Re: Good code or bad code? [message #170190 is a reply to message #170185] |
Sun, 17 October 2010 13:25 |
MikeB
Messages: 65
Registered: September 2010
Karma: 0
|
Member |
|
|
Hamish Campbell wrote:
> On Oct 17, 5:20 pm, MikeB<mpbr...@gmail.com> wrote:
>> I'm mostly playing with PHP to get a feeling for coding in it.
>>
>> So as part of all this URI/URL/redirect stuff I spent some time looking
>> at the contents of $_SERVER and I came up with this code to find the
>> filename of the file I'm invoked from. Now one condition that I wanted
>> to cater for was if the filename had multiple "."s in the name, for
>> instance myfile.inc.php, or something like this.
>>
>> So I came up with this piece of code.
>>
>> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
>> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
>> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>>
>> So I was wondering if that is good code or if I could have written it
>> better, since looking at it it is quite hard to understand.
>
> Have a look at:
>
> <http://nz.php.net/manual/en/function.pathinfo.php>
>
> e.g:
>
> $name = pathinfo($_SERVER['REQUEST_URI'], PATHINFO_FILENAME);
>
Thank you, that is *much* cleaner and more understandable.
|
|
|
|
| Re: Good code or bad code? [message #170191 is a reply to message #170189] |
Sun, 17 October 2010 13:37 |
Jerry Stuckle
Messages: 2598
Registered: September 2010
Karma: 0
|
Senior Member |
|
|
On 10/17/2010 9:12 AM, MikeB wrote:
> Jerry Stuckle wrote:
>> On 10/17/2010 12:20 AM, MikeB wrote:
>>> I'm mostly playing with PHP to get a feeling for coding in it.
>>>
>>> So as part of all this URI/URL/redirect stuff I spent some time looking
>>> at the contents of $_SERVER and I came up with this code to find the
>>> filename of the file I'm invoked from. Now one condition that I wanted
>>> to cater for was if the filename had multiple "."s in the name, for
>>> instance myfile.inc.php, or something like this.
>>>
>>> So I came up with this piece of code.
>>>
>>> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
>>> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
>>> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>>>
>>> So I was wondering if that is good code or if I could have written it
>>> better, since looking at it it is quite hard to understand.
>>>
>>> Thanks
>>> MikeB
>>>
>>> I'll go away again for a while after this, I probably have been relying
>>> on all y'alls good graces too much again.
>>
>> In addition to what Hammish said, this information is sent by the
>> browser and cannot be trusted. Some browsers may not send it, and if it
>> is sent, it may be falsified (i.e. by a hacker).
>>
>
> I said I would shut up for a while, but now you bring up something else.
>
> I wrote that code to find the fiilename (eg. index) so that I could
> dynamically derive the name of an accompanying template file.
>
> So if I'm running from index.php, I could derive index.tpl for a Smarty
> template to accompany the php file.
>
> If a hacker falsifies this, the template won't match the php file
> creating the output and the page (s)he sees will be all messed up.
>
> so that brings up two questions:
>
> 1. Is there a better way to dynamically derive a base filename? Eg. Is
> there a php function that I can use to get the name of the executing
> file? That may be better/safer then.
>
> 2. Is the risk of this being hacked sufficient that I should rather
> statically code the template filename and then go through the hassle of
> recoding the name every time I change (or move) the base file around?
>
> I'm not sure that there is an risk to a website if the Smarty template
> gets messed up, but I can see that there might be other uses that could
> me more risky, so I'll certainly bear that in mind.
I think we have a confusion of terminology here. The script is "invoked
from " another page, typically by an anchor tag link - although there
are other ways (i.e. flash, javascript, etc.).
The current script being run can be executed in a number of ways; it may
be called directly or it may be included by other scripts, for instance.
But the REQUEST_URI is not a reliable indicator - it does show the page
requested from the server, but the web server's configuration may
significantly change this. For instance, in some CMS's, all requests
are handled by a single script, but each page has it's own URI. The
Apache mod_rewrite changes the request to invoke the single script with
specific parameters.
So in this case it can be correct - but only if you also know the web
server's configuration. And a change to that configuration can cause
problems.
--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex(at)attglobal(dot)net
==================
|
|
|
|
| Re: Good code or bad code? [message #170193 is a reply to message #170189] |
Sun, 17 October 2010 17:09 |
Thomas 'PointedEars'
Messages: 701
Registered: October 2010
Karma: 0
|
Senior Member |
|
|
MikeB wrote:
> Jerry Stuckle wrote:
>> On 10/17/2010 12:20 AM, MikeB wrote:
>>> So as part of all this URI/URL/redirect stuff I spent some time looking
>>> at the contents of $_SERVER and I came up with this code to find the
>>> filename of the file I'm invoked from. Now one condition that I wanted
>>> to cater for was if the filename had multiple "."s in the name, for
>>> instance myfile.inc.php, or something like this.
>>>
>>> So I came up with this piece of code.
>>>
>>> $uriParts = explode("/",$_SERVER['REQUEST_URI']);
>>> $thisFile = substr(end($uriParts),0,(strlen(end($uriParts))) -
>>> (strlen(end($uriParts))- strrpos(end($uriParts),'.')));
>>>
>>> So I was wondering if that is good code or if I could have written it
>>> better, since looking at it it is quite hard to understand.
>>>
>>> I'll go away again for a while after this, I probably have been relying
>>> on all y'alls good graces too much again.
>>
>> In addition to what Hammish said, this information is sent by the
>> browser and cannot be trusted. Some browsers may not send it, and if it
>> is sent, it may be falsified (i.e. by a hacker).
Bullshit/FUD. $_SERVER['REQUEST_URI'] yields the HTTP request URI, e.g.
http://foo.example/bar?baz
for an HTTP request containing the headers
GET /bar?baz HTTP/1.1
Host: foo.example
As the array name indicates, the value is provided by the Web *server* that
is running the executing PHP instance, and is completely independent of the
(capabilities of the) HTTP client application (e.g., the browser) that made
the request.
> I wrote that code to find the fiilename (eg. index) so that I could
> dynamically derive the name of an accompanying template file.
>
> So if I'm running from index.php, I could derive index.tpl for a Smarty
> template to accompany the php file.
Use $_SERVER['SCRIPT_NAME'], since $_SERVER['PHP_SELF'] can be misused for
code injection:
<http://en.wikipedia.org/wiki/Cross-site_scripting>
RTFM and call phpinfo() for details on $_SERVER.
PointedEars
--
Prototype.js was written by people who don't know javascript for people
who don't know javascript. People who don't know javascript are not
the best source of advice on designing systems that use javascript.
-- Richard Cornford, cljs, <f806at$ail$1$8300dec7(at)news(dot)demon(dot)co(dot)uk>
|
|
|
|
|
|
| Re: Good code or bad code? [message #170198 is a reply to message #170196] |
Sun, 17 October 2010 18:39 |
Thomas 'PointedEars'
Messages: 701
Registered: October 2010
Karma: 0
|
Senior Member |
|
|
Magno wrote:
> Thomas 'PointedEars' Lahn wrote:
>> Bullshit/FUD. $_SERVER['REQUEST_URI'] yields the HTTP request URI, e.g.
>>
>> http://foo.example/bar?baz
>>
>> for an HTTP request containing the headers
>>
>> GET /bar?baz HTTP/1.1
>> Host: foo.example
>
> No. It shows the URI relative to the domain root. not including the
> domain name.
ACK, my bad. Yet that is even more proof that it is nonsense to claim that
browsers do not need to send it, and that it could be forged. A HTTP
request *requires* a request URI-reference. And for the PHP script to be
triggered, that URI-reference must, in the end, be correct.
>> [...]
>>
>> Use $_SERVER['SCRIPT_NAME'], since $_SERVER['PHP_SELF'] can be misused
>> for code injection:
>>
>> <http://en.wikipedia.org/wiki/Cross-site_scripting>
>
> That is not true.
> If you think it is true, give us an example of abusing it for code
> injection.
The correct course of action would be for you to present an argument why my
statement is not true.
Anyhow, for an oft-cited (and thus easily found) example (here: courtesy of
<http://blog.oncode.info/>, slightly adapted), take this problematic, but
often found, `form' element:
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
…
</form>
and this URI to trigger the PHP script containing it:
http://foo.example/bar/myform.php/%22%3E%3C%2Fform%3EHier%20ein%20Javascrip t%3A%20%3Cscript%20type%3D%22text%2Fjavascript%22%3Ewindow.alert('Gotcha!')%3B%3C%2Fscript%3E%3Cform%20action%3D%22%2Fcontact%2Fmyform.php
(Yes, wrapping $_SERVER['PHP_SELF'] in htmlentities() or htmlspecialchars()
would help here, but $_SERVER['SCRIPT_NAME'] usually does not require to be
wrapped in either one. Hence my recommendation.)
>> RTFM and call phpinfo() for details on $_SERVER.
>
> What the OP should read is.-
>
> http://php.net/manual/en/reserved.variables.server.php
That *is* the FM.
> and do a print_r($_SERVER);
That is what phpinfo() shows, among many other values.
PointedEars
--
Anyone who slaps a 'this page is best viewed with Browser X' label on
a Web page appears to be yearning for the bad old days, before the Web,
when you had very little chance of reading a document written on another
computer, another word processor, or another network. -- Tim Berners-Lee
|
|
|
|
| Re: Good code or bad code? [message #170200 is a reply to message #170198] |
Sun, 17 October 2010 22:16 |
Magno
Messages: 49
Registered: October 2010
Karma: 0
|
Member |
|
|
On 10/17/2010 03:39 PM, Thomas 'PointedEars' Lahn wrote:
> The correct course of action would be for you to present an argument why my
> statement is not true.
>
> Anyhow, for an oft-cited (and thus easily found) example (here: courtesy of
> <http://blog.oncode.info/>, slightly adapted), take this problematic, but
> often found, `form' element:
>
> <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
> …
> </form>
>
> and this URI to trigger the PHP script containing it:
>
> http://foo.example/bar/myform.php/%22%3E%3C%2Fform%3EHier%20ein%20Javascrip t%3A%20%3Cscript%20type%3D%22text%2Fjavascript%22%3Ewindow.alert('Gotcha!')%3B%3C%2Fscript%3E%3Cform%20action%3D%22%2Fcontact%2Fmyform.php
>
> (Yes, wrapping $_SERVER['PHP_SELF'] in htmlentities() or htmlspecialchars()
> would help here, but $_SERVER['SCRIPT_NAME'] usually does not require to be
> wrapped in either one. Hence my recommendation.)
I use to assume everyone being wise enough to not do such an idiotic
mistakes like not filtering what you are going to print on HTML.
You must ALWAYS use htmlspecialchars, when the user interaction can
alter anything you will print in the output.
>>> RTFM and call phpinfo() for details on $_SERVER.
>>
>> What the OP should read is.-
>>
>> http://php.net/manual/en/reserved.variables.server.php
>
> That *is* the FM.
Didn’t say it is not. Anyway there are more respectful ways for you to
tell that to someone, other than your typical condescendence.
|
|
|
|
| Re: Good code or bad code? [message #170201 is a reply to message #170200] |
Sun, 17 October 2010 23:58 |
Thomas 'PointedEars'
Messages: 701
Registered: October 2010
Karma: 0
|
Senior Member |
|
|
Magno wrote:
> On 10/17/2010 03:39 PM, Thomas 'PointedEars' Lahn wrote:
>> Anyhow, for an oft-cited (and thus easily found) example (here: courtesy
>> of <http://blog.oncode.info/>, slightly adapted), take this problematic,
>> but often found, `form' element:
>>
>> <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
>> …
>> </form>
>>
>> and this URI to trigger the PHP script containing it:
>>
>>
http://foo.example/bar/myform.php/%22%3E%3C%2Fform%3EHier%20ein%20Javascrip t%3A%20%3Cscript%20type%3D%22text%2Fjavascript%22%3Ewindow.alert('Gotcha!')%3B%3C%2Fscript%3E%3Cform%20action%3D%22%2Fcontact%2Fmyform.php
>>
>> (Yes, wrapping $_SERVER['PHP_SELF'] in htmlentities() or
>> htmlspecialchars() would help here, but $_SERVER['SCRIPT_NAME'] usually
>> does not require to be
>> wrapped in either one. Hence my recommendation.)
>
> I use to assume everyone being wise enough to not do such an idiotic
> mistakes like not filtering what you are going to print on HTML.
(Fallacies: Raising the bar / No true Scotsman)
You would be surprised how often this pattern has occurred in the past and
is occurring now, because people simply do not know this about $PHP_SELF or
$_SERVER['PHP_SELF']. STFW.
> You must ALWAYS use htmlspecialchars, when the user interaction can
> alter anything you will print in the output.
Not when, iff. In the case of $_SERVER['SCRIPT_NAME'], user interaction
cannot alter anything. That is my point that you are still missing.
PointedEars
--
Danny Goodman's books are out of date and teach practices that are
positively harmful for cross-browser scripting.
-- Richard Cornford, cljs, <cife6q$253$1$8300dec7(at)news(dot)demon(dot)co(dot)uk> (2004)
|
|
|
|
| Re: Good code or bad code? [message #170204 is a reply to message #170200] |
Mon, 18 October 2010 00:06 |
Jerry Stuckle
Messages: 2598
Registered: September 2010
Karma: 0
|
Senior Member |
|
|
On 10/17/2010 6:16 PM, Magno wrote:
> On 10/17/2010 03:39 PM, Thomas 'PointedEars' Lahn wrote:
>> The correct course of action would be for you to present an argument
>> why my
>> statement is not true.
>>
>> Anyhow, for an oft-cited (and thus easily found) example (here:
>> courtesy of
>> <http://blog.oncode.info/>, slightly adapted), take this problematic, but
>> often found, `form' element:
>>
>> <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
>> …
>> </form>
>>
>> and this URI to trigger the PHP script containing it:
>>
>> http://foo.example/bar/myform.php/%22%3E%3C%2Fform%3EHier%20ein%20Javascrip t%3A%20%3Cscript%20type%3D%22text%2Fjavascript%22%3Ewindow.alert('Gotcha!')%3B%3C%2Fscript%3E%3Cform%20action%3D%22%2Fcontact%2Fmyform.php
>>
>>
>> (Yes, wrapping $_SERVER['PHP_SELF'] in htmlentities() or
>> htmlspecialchars()
>> would help here, but $_SERVER['SCRIPT_NAME'] usually does not require
>> to be
>> wrapped in either one. Hence my recommendation.)
>
> I use to assume everyone being wise enough to not do such an idiotic
> mistakes like not filtering what you are going to print on HTML.
>
> You must ALWAYS use htmlspecialchars, when the user interaction can
> alter anything you will print in the output.
>
>>>> RTFM and call phpinfo() for details on $_SERVER.
>>>
>>> What the OP should read is.-
>>>
>>> http://php.net/manual/en/reserved.variables.server.php
>>
>> That *is* the FM.
>
> Didn’t say it is not. Anyway there are more respectful ways for you to
> tell that to someone, other than your typical condescendence.
He doesn't know any other way - because he can't say anything intelligent.
He's a well known troll on several Usenet newsgroups besides this one.
--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex(at)attglobal(dot)net
==================
|
|
|
|
| Re: Good code or bad code? [message #170205 is a reply to message #170201] |
Mon, 18 October 2010 06:09 |
Hamish Campbell
Messages: 15
Registered: September 2010
Karma: 0
|
Junior Member |
|
|
On Oct 18, 12:58 pm, Thomas 'PointedEars' Lahn <PointedE...@web.de>
wrote:
>> You must ALWAYS use htmlspecialchars, when the user interaction can
>> alter anything you will print in the output.
>
> Not when, iff. In the case of $_SERVER['SCRIPT_NAME'], user interaction
> cannot alter anything. That is my point that you are still missing.
Always filter untrusted content for output. Untrusted is anything you
have not set yourself or has not been demonstrably cleaned for output.
|
|
|
|
Calendar
Search
Help
Members
Register
Login
Home
]