WooCommerce.NET icon indicating copy to clipboard operation
WooCommerce.NET copied to clipboard

Query string options are automatically percent encoded over HTTP, but not over HTTPS

Open gaazkam opened this issue 4 years ago • 0 comments

Make sure you have included the below details when open an issue. Thank you.

  • Wordpress version, WooCommerce version and WooCommerce.NET version

WP 5.7.2, WC 5.3.0, WC.NET 0.8.3

  • Steps to replicate the issue

Make a request using the below sample code (assuming your Wordpress installation is under localhost/wordpress):

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using WooCommerceNET;
using WooCommerceNET.WooCommerce.v3;

namespace ConsoleApp3
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var wc = new WCObject(new RestAPI
            (
                "http://localhost/wordpress/wp-json/wc/v3/",
                "redacted",
                "redacted",
                requestFilter: request =>
                {
                    request.UserAgent = "TestException";
                    request.Accept = "application/json";
                }
            ));

            var after = new DateTimeOffset(year: 2021, month: 5, day: 16, hour: 19, minute: 22, second: 7, offset: new TimeSpan(0)).ToString("o");

            Console.WriteLine(after);

            await wc.Order.GetAll(new Dictionary<string, string>
            {
                {"page", "1" },
                {"per_page", "100" },
                {"after", after }
            });

            Console.WriteLine("done");
            Console.ReadLine();
        }
    }
}

Then edit the above code replacing http with https and re-run the test.

  • Details of the error message if there is any

The value of the query string after parameter contains the + sign, which is required to be percent-encoded.

Inspecting the server access logs reveals that this only happens over http, but not over https. Compare:

127.0.0.1 - - [18/May/2021:16:06:38 +0200] "GET /wordpress/wp-json/wc/v3/orders?oauth_consumer_key=redacted&oauth_nonce=redacted&oauth_signature_method=HMAC-SHA256&oauth_timestamp=1621346795&oauth_version=1.0&page=1&per_page=100&after=2021-05-16T19%3A22%3A07.0000000%2B00%3A00&oauth_signature=redacted HTTP/1.1" 200 2 "-" "TestException"

with:

127.0.0.1 - redacted [18/May/2021:16:06:51 +0200] "GET /wordpress/wp-json/wc/v3/orders?page=1&per_page=100&after=2021-05-16T19:22:07.0000000+00:00 HTTP/1.1" 400 213

In the latter case the value of the after parameter is malformed, causing the server to return the HTTP 400 status code and the following exception is thrown in the C# code:

System.Net.WebException: '{"code":"rest_invalid_param","message":"Invalid parameter(s): after","data":{"status":400,"params":{"after":"Invalid date."},"details":{"after":{"code":"rest_invalid_date","message":"Invalid date.","data":null}}}}'

Suggestion

I suggest that values of query string parameters should be automatically encoded over http as well as https.

Doing so would be a breaking change however, since it is likely that most existing production code connects to WooCommerce instances over https, and such code must manually encode certain query string parameters (like after) as shown below. If this is an issue, then perhaps query string parametrs should not be automatically encoded over http nor over https?

However, whether the values are encoded or not, I belive that the behavior should be consistent. The current situation is confusing and cumbersome to fully remedy.

Workaround

The solution for https is to manually encode the values of the options dictionary:

var options = new Dictionary<string, string>
{
    {"page", "1" },
    {"per_page", "100" },
    {"after", after }
};
await wc.Order.GetAll(options.ToDictionary(kv => Uri.EscapeDataString(kv.Key), kv => Uri.EscapeDataString(kv.Value)));

This will make requests over https work. However, this will also break requests over http by doubly encoding them.

Therefore, code must only escape the values of the dictionary if the requests go over https, but it must not escape them if the requests go over http.

gaazkam avatar May 18 '21 14:05 gaazkam