1
\$\begingroup\$

I wanted to allow my website visitors (any Tom, Dick & Harry) submit their links to my webpage for output on my page. I thought it's best to parse user submitted urls before echoing their submitted urls on my page. Best to parse the urls as I won't know what urls they will be submitting nor the structures of their urls.

A user could theoretically visit my page and inject some Javascript code using, for example:

?search=<script>alert('hacked')</script>

You understand my point.

I needed to write php script that when users submit their urls, then my php script parses their urls and encodes them by adding urlencode, rawurlencode, intval in the appropriate places before outputting them via htmlspecialchars. Another wrote this following script. Problem is, it outputs like so:

http%3A%2F%2Fexample.com%2Fcat%2Fsubcat?var_1=value+1&var2=2&this_other=thing&number_is=13

It should output like this:

http://example.com/cat/subcat?var_1=value+1&var2=2&this_other=thing&number_is=13

NOTE:

$url = $_POST[url]; //User Submitted Url.

This is their code .... Third Party Code:

function encodedUrl($url){
    $query_strings_array = [];
    $query_string_parts  = [];
    // parse URL & get query
    $scheme        = parse_url($url, PHP_URL_SCHEME);
    $host          = parse_url($url, PHP_URL_HOST);
    $path          = parse_url($url, PHP_URL_PATH);
    $query_strings = parse_url($url, PHP_URL_QUERY);

    // parse query into array
    parse_str($query_strings, $query_strings_array);

    // separate keys & values
   $query_strings_keys   = array_keys($query_strings_array);
   $query_strings_values = array_values($query_strings_array);

   // loop query
  for($i = 0; $i < count($query_strings_array); $i++){
       $k   = urlencode($query_strings_keys[$i]);
       $v   = $query_strings_values[$i];
       $val = is_numeric($v) ? intval($v) : urlencode($v);
    
       $query_string_parts[] = "{$k}={$val}";
   }

   // re-assemble URL
   $encodedHostPath = rawurlencode("{$scheme}://{$host}{$path}");

   return $encodedHostPath . '?' . implode('&', $query_string_parts);
}

$url1 = 'http://example.com/cat/subcat?var 1=value 1&var2=2&this other=thing&number is=13';
$url2 = 'http://example.com/autos/cars/list.php?state=california&max_price=50000';

// run urls thru function & echo
// run urls thru function & echo
echo $encoded_url1 = encodedUrl($url1); echo '<br>'; 
echo $encoded_url2 = encodedUrl($url2); echo '<br>'; 
?>

So, I changed following of their's:

$encodedHostPath = rawurlencode("{$scheme}://{$host}{$path}");

to this of mine (my amendment):

$encodedHostPath = rawurlencode("{$scheme}").'://'.rawurlencode("{$host}").$path;

And it seems to be working. As it's outputting:

http://example.com/cat/subcat?var_1=value+1&var2=2&this_other=thing&number_is=13

Q1A. But I am not sure if I put the raw_urlencode() in the right places or not and so best you check.

Q1B. Also, wondering should not the $path be inside raw_urlencode like so ?

raw_urlencode($path)

Note however that, it doesn't output the links right.

Q2. Anyway, I'm wondering whether I should add htmlspecialchars() or not to output the user submitted links. So wondering whether or not to change their following line:

// run urls thru function & echo
echo $encoded_url1 = encodedUrl($url1); echo '<br>'; 

// run urls thru function & echo
echo $encoded_url2 = encodedUrl($url2); echo '<br>'; 
?>

to this (my amendment) instead:

// run urls thru function & echo
echo $link1 = '<a href=' .htmlspecialchars($encoded_url1) .'>' .htmlspecialchars($encoded_url1) .'</a>';
echo '<br/>';

// run urls thru function & echo
echo $link2 = '<a href=' .htmlspecialchars($encoded_url2) .'>' .htmlspecialchars($encoded_url2) . '</a>';
echo '<br>';

I'd appreciate anyone editing my code by adding any necessary parts that I missed (htmlspecialchars(), intval(), urlencode(), raw_urlencode(), etc. and deleting unnecessary parts.

\$\endgroup\$
5
  • \$\begingroup\$ So are we mostly reviewing third party code? \$\endgroup\$ Sep 24, 2021 at 23:24
  • \$\begingroup\$ @mickmacusa, third parties on forum gave code to help me out but they had flaws. When I asked them to fix they all went silent. Then it took me weeks to fix. Now, you are reviewing my fix. If you say the fix is ok then I take your word for it. Else, I hesitate to accept my fix. I still a beginner level student at procedural style programming. Not at oop yet. Yes, most of the code was written by third party in the programming forum. I mentioned above, what I amended as the fix. \$\endgroup\$ Sep 26, 2021 at 20:12
  • \$\begingroup\$ My suggestion is to write unit tests for each case, then either verify that your code already passes each test, or modify the code so that it does. \$\endgroup\$
    – CJ Dennis
    Oct 2, 2021 at 2:12
  • \$\begingroup\$ @CJ Dennis, So you didn't review it then ? \$\endgroup\$ Oct 4, 2021 at 16:55
  • \$\begingroup\$ The problem is that this site is for working code that you want to refactor, not for fixing incorrect behaviour. If you want a code review, stay here. If you want to fix your code, ask your question on Stack Overflow. \$\endgroup\$
    – CJ Dennis
    Oct 4, 2021 at 22:13

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.