2

We're just upgrading to form v3.0 and whilst doing so, refactoring our code.

Whilst doing so, we noticed that when using http_build_query which takes an associative array and converts it into an RFC1738 valid URL, that SagePay fails with the following error:

The SuccessURL format is invalid

The form submitting to the SagePay endpoint has an enctype of application/x-www-form-urlencoded.

However... If we manually build the string to encrypt by doing:

$tmp = '';
foreach ($crypt_store as $key => $value) {
    $tmp .= sprintf('&%s=%s', $key, $value);
}

It works...

Now as I understand RFC1738, if an url exists within an url, it should be encoded, i.e.

RFC1738:

&VendorTxCode=Test&SuccessUrl=http%3A%2F%2Fwww.stackoverflow.com%3Fa%3Da%26b%3Db&FailureUrl...

SagePay:

&VendorTxCode=Test&SuccessUrl=http://www.stackoverflow.com?a=a&b=b&FailureUrl...

Surely if SagePay are following RFC1738, encoding the URL should work? Or is it because the string is encrypted which means it doesn't really matter?

Any thoughts?

Thanks

Gavin

Gavin
  • 6,284
  • 5
  • 30
  • 38

1 Answers1

1

You are correct. Because the Success / Failure URLs are encrypted within the Crypt field, there is no need to encode them.

Rik Blacow
  • 1,139
  • 1
  • 7
  • 12
  • Except when they contain `&` or other special characters? This especially blows up when the URL contains parameters that match those used in crypt field. (From my testing and your answer, I assume you split the string on known parameter names rather than a more standard split on `&`). – Deanna Jul 21 '15 at 12:42
  • 1
    Correct - so if your SuccessURL string contains &VendorTxCode= it's going to get removed. – Rik Blacow Jul 21 '15 at 14:22
  • @RikBlacow That's kinda ridiculous... The request body follows the "urlencoded-like" format, but we cannot actually escape the values according to the industry-standard RFCs. I'm just working with some legacy code and was really surprised when the request got broken (error 5080) after applying proper escaping to the user-provided data. If I'm supposed to leave it just as is, isn't it a security hole? – volvpavl Oct 13 '17 at 10:50