verbb / consume

A Craft CMS plugin to create HTTP requests in your Twig templates to consume JSON, XML or CSV content.
Other
4 stars 1 forks source link

Consume malforms request URL #7

Closed jannisborgers closed 7 months ago

jannisborgers commented 7 months ago

Describe the bug

In both generic ad-hoc clients and clients set up in the admin panel, errors are thrown because the request URL gets malformed.

For example, this call:

{% set paypal_order = consume("paypal_client", "POST", "/v2/checkout/orders", {
  json: paypal_query
}) %}

results in the following error in Consume’s logs:

[ERROR] Unable to fetch data: “cURL error 6: Could not resolve host: api-m.sandbox.paypal.comv2 (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api-m.sandbox.paypal.comv2/checkout/orders”

(notice comv2 in the URL).

I also get this error with a ad-hoc client. This setup:

{% set test_client = {
  base_uri: "https://some-domain.com/xml/v18.0/",
} %}

{# set up order query here… #}

{% set order = consume(test_client, "POST", "api.php", {
  query: order_query
}) %}

results in an error where Consume strips the /v18.0/ segment completely, making a call to https://some-domain.com/xml/api.php instead. I could solve this one by passing an empty string as the endpoint and passing api.php as part of the base_uri instead. But for Paypal, this isn’t possible due to the way it excepts the request.

I updated to the latest version of Consume and Auth today.

I use Consume 1.0.5 and Auth 1.0.12, but used Consume "dev-craft-4 as 1.0.2" and Auth "dev-craft-4 as 1.0.5" before.

Steps to reproduce

  1. Set up client
  2. Use client with multiple segments in base_uri and endpoint.

Craft CMS version

4.5.12

Plugin version

1.0.5

Multi-site?

No

Additional context

No response

engram-design commented 7 months ago

I believe I see the issue with the Auth module Base URI normalisation - if you want to just triple-check that on your end? To get this early, run composer require verbb/auth:"dev-craft-4 as 1.0.12".

As for the ad-hoc, non-OAuth providers, I can't seem to replicate that. I've performed the following tests for ad-hoc:

{# Base URI with trailing slash, endpoint with leading slash #}
{% set client = { base_uri: 'https://jsonplaceholder.typicode.com/' } %}
{% set data = consume(client, 'GET', '/todos/1') %}

{# Base URI with trailing slash, endpoint without leading slash #}
{% set client = { base_uri: 'https://jsonplaceholder.typicode.com/' } %}
{% set data = consume(client, 'GET', 'todos/1') %}

{# Base URI without trailing slash, endpoint with leading slash #}
{% set client = { base_uri: 'https://jsonplaceholder.typicode.com' } %}
{% set data = consume(client, 'GET', '/todos/1') %}

{# Base URI without trailing slash, endpoint without leading slash #}
{% set client = { base_uri: 'https://jsonplaceholder.typicode.com' } %}
{% set data = consume(client, 'GET', 'todos/1') %}

I'm also just a test JSON endpoint, just so it's easier to replicate.

I've also created two control panel based clients with the URL of https://jsonplaceholder.typicode.com and https://jsonplaceholder.typicode.com/

{% set data = consume('test1', 'GET', 'todos/1') %}
{% set data = consume('test2', 'GET', '/todos/1') %}

And both produce the same result. Just wanted to check we're on the same page on that one?

scandella commented 7 months ago

All of a sudden, I've got the same problem! This call

{% set firstData = craft.consume.fetchData('googleDrive', 'GET', 'drive/v3/files/', {
            query: {
            trashed:false,
            orderBy: 'modifiedTime desc'
}
}) %}

raise an error: Unable to fetch data: “cURL error 6: Could not resolve host: www.googleapis.comdrive (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://www.googleapis.comdrive/v3/files/?trashed=0&orderBy=modifiedTime+desc”

Seems to miss an "/" in the url path.

Updating to verbb/auth:"dev-craft-4 as 1.0.12" fixes the problem.

engram-design commented 7 months ago

Fixed in 1.0.6

jannisborgers commented 7 months ago

Confirming that the PayPal request goes through after updating to Consume 1.0.6 and Auth 1.0.13. 👍 Thanks a lot!

jannisborgers commented 7 months ago

My ad-hoc client still requests to https://some-domain.com/xml/api.php, dropping the /v18.0/ segment, both when using a trailing slash in the base_uri and a slash at the beginning of the endpoint parameter. I suspected it could be the . character in the API versioning, but removing it and just using /v18/ still calls the URL without the segment.

jannisborgers commented 7 months ago

Dug one step further and found another weird thing: using the following setup:

{% set test_client = {
  base_uri: "https://some-domain.net/",
} %}

{# set up api_settings_query #}

{% set api_settings = consume(test_client, "POST", "/xml/v19.0/api.php", {
  query: api_settings_query
}) %}

The request goes to https://some-domain.net/api.php, so removing all segments. Is this by design? I always considered both parts to be concatenated together no matter where I decide to cut the base_uri.

engram-design commented 7 months ago

Ah yes, I believe it's our logic of being able to figure out if you're calling a file or not, so you're spot-on with the hunch on the . character. We can actually completely remove this unnecessary normalization now.

Should be fixed in 1.0.7

jannisborgers commented 7 months ago

Sorry @engram-design — no dice. I updated Consume to 1.0.7 and Auth to 1.0.15, and moved everything back to this:

{% set test_client = {
  base_uri: "https://some-domain.net/xml/v19.0/",
} %}

{# set up api_settings_query #}

{% set api_settings = consume(test_client, "POST", "api.php", {
  query: api_settings_query
}) %}

The resulting request is logged as https://some-domain.net/xml/api.php, removing the versioning segment. I can work around this because the endpoint is always the same for this API, everything else is passed as a query parameter. For other APIs it might be more problematic.

engram-design commented 7 months ago

Sorry about that, I believe that should be fixed now in 1.0.8

jannisborgers commented 7 months ago

Thanks Josh, that did the trick 👍