badgateway / ketting

The HATEOAS client for javascript
https://www.npmjs.com/package/ketting
MIT License
551 stars 32 forks source link

Support HAL Forms actions with no request body #453

Open sazzer opened 2 years ago

sazzer commented 2 years ago

Unless I'm missing something, there's no way to specify a HAL Forms _template entry that doesn't send a request body. For example, to indicate in a HAL document that you can perform some action by doing a POST to some URI without a request body.

I can fake this using _links but it doesn't really feel right - they are meant to define links to other related resources, not actions that can be performed on this one.

For example, a User resource might look like this:

{
  "name": "Test User",
  "email": "testuser@example.com",
  "_links": {
    "self": {
      "href": "/users/auth0%7C6240a09cc665610070aa5bfb"
    }
  },
  "_templates": {
    "edit": {
      "method": "PUT",
      "fields": [
        {
          "name": "name",
          "type": "text",
          "required": true
        }, {
          "name": "email",
          "type": "email",
          "required": true
        }
      ]
    },
    "/users/actions/change_password": {
      "target": "/users/auth0%7C6240a09cc665610070aa5bfb/change_password",
      "method": "POST"
    },
    "/users/actions/verify_email": {
      "target": "/users/auth0%7C6240a09cc665610070aa5bfb/verify_email",
      "method": "POST"
    }
  }
}

In this case, both the /users/actions/change_password and /users/actions/verify_email templates define actions that are performed on this resource, but they don't need a request body. Instead they are simply a POST to the given URI that triggers the action and returns the result.

Doing the above will cause the action to send an application/json request with a request body of {}. Explicitly setting the contentType property on the template to null has no effect either.

Further, because of the TS definition of Action.submit() I'm forced to pass in some payload when calling this method, so it seems that actions without a body are just not supported right now.

sazzer commented 2 years ago

I should mention that I only noticed this because Prism returns an HTTP 415 if the request has a body but the OpenAPI spec doesn't support one. Most server frameworks likely won't care about this.

mamund commented 2 years ago

the HAL-FORMS specification (here: https://rwcbook.github.io/hal-forms/) accounts for the ability to send body-less unsafe requests (see: https://rwcbook.github.io/hal-forms/#_code_properties_code).

FWIW, the HAL-FORMS example you show doesn't comply w/ the linked spec since the HAL-FORMS spec calls for a properties collection, not a fields collection. might that be a typo for this example?

IOW, compliant implementations of the HAL-FORMS spec should allow for body-less HTTP POST.

sazzer commented 2 years ago

That's a typo. I was converting it from Siren, which uses the "fields" term, because HAL Forms has inbuilt support for required and readonly.

On Sun, 2 Oct 2022, 17:10 Mike Amundsen, @.***> wrote:

the HAL-FORMS specification (here: https://rwcbook.github.io/hal-forms/) accounts for the ability to send body-less unsafe requests (see: https://rwcbook.github.io/hal-forms/#_code_properties_code).

FWIW, the HAL-FORMS example you show doesn't comply w/ the linked spec since the HAL-FORMS spec calls for a properties collection, not a fields collection. might that be a typo for this example? or is there another spec for FORMS in HAL that is in use here?

— Reply to this email directly, view it on GitHub https://github.com/badgateway/ketting/issues/453#issuecomment-1264678179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQEGBB76XSLJKBAZI6RI3WBGXWVANCNFSM6AAAAAAQ23CFAE . You are receiving this because you authored the thread.Message ID: @.***>

mamund commented 2 years ago

no problem. there are a couple "flavors" of HAL-FORMS out there, so i was just ticking a box there.

the real convo is about the body-less POST. the spec was meant to support that. I assume the spec is OK but implementations may not be fully-compliant.

evert commented 2 years ago

Given:

If the contentType property is missing, is set to empty, or contains an unrecognized value, the client SHOULD act is if the contentType is set to "application/json".

And:

If the array is missing or empty, the properties collection MUST be treated as an empty set of parameters 

This tells me that we need to send valid application/json.

The 'set' of properties in JSON normally looks like this:

{
  "prop1": 1,
  "prop2": 2
}

So an empty set makes more sense as:

{
}

Than an empty string, especially given that an empty strong is not valid json

sazzer commented 2 years ago

Both from HAL Forms? Maybe the problem is in that instead of Ketting then 🙂

As I said earlier, it's only really a problem because Prism rejects the requests as having an unexpected content type in the request body...

On Sun, 2 Oct 2022, 18:29 Evert Pot, @.***> wrote:

Given:

If the contentType property is missing, is set to empty, or contains an unrecognized value, the client SHOULD act is if the contentType is set to "application/json".

And:

If the array is missing or empty, the properties collection MUST be treated as an empty set of parameters

This tells me that we need to send valid application/json, and an empty set makes the most sense as {}.

— Reply to this email directly, view it on GitHub https://github.com/badgateway/ketting/issues/453#issuecomment-1264693986, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQEGGE2J57GGDVZ2XVVM3WBHA6HANCNFSM6AAAAAAQ23CFAE . You are receiving this because you authored the thread.Message ID: @.***>

evert commented 2 years ago

Yeah those are both excerpts =)

You could try application/x-www-form-urlencoded instead, as an empty set there is a 0-byte string!

sazzer commented 2 years ago

When not specifying a content type at all, Prism gives:

image

And the Firefox console shows:

image
evert commented 2 years ago

I don't really know what Prism is, so not sure if that's normal :P is there anything else I could potentially do here?

sazzer commented 2 years ago

Interestingly - and I'm surprised by this - specifying "contentType": "application/x-www-form-urlencoded" in the HAL Template and it does work. :)

I really expected that to fail on the exact same error, since the Prism error was "Invalid content type", and this is still specifying a content-type header even if there's now no payload.

sazzer commented 2 years ago

I don't really know what Prism is, so not sure if that's normal :P is there anything else I could potentially do here?

Sorry! Prism is this: https://stoplight.io/open-source/prism It's just a quick way of getting a server to produce responses for requests without yet having written a real server :) The thing is that it also asserts that incoming requests are valid against the spec, and rejects them if not. So when the incoming request had a content-type header and a request body, when the spec says that it shouldn't do, Prism was rejecting it as invalid. Which is fair, since there's no need for a request body in the first place, but was annoying since I assumed - incorrectly! - that it was a Ketting issue that a request body was being sent in the first palce.

mamund commented 2 years ago

I suspect it's a rule in prism that blindly attempts to parse a JSON object body when the content-type: application/json header is passed during some HTTP methods (e.g. POST/PUT/PATCH) -- even if no body is included.

sazzer commented 2 years ago

Maybe. Regardless though, it seems that Ketting is working correctly to the HAL Forms spec - just that HAL Forms itself doesn't account for this use case - and I've got a workaround to make Prism happy enough until I actually get a real server working with these responses. So unless you have some desire to allow Ketting to support bodyless action requests this can be closed :)

evert commented 2 years ago

Personally I think if a Content-Type is set to application/json and no body is set, that should be treated as invalid.

sazzer commented 2 years ago

It was sending a body though.

The HTTP request was:

POST /users/auth0%7C6240a09cc665610070aa5bfb/verify_email HTTP/1.1
Host: localhost:4010
User-Agent: Ketting/7.5.1
Accept: application/prs.hal-forms+json;q=1.0, application/hal+json;q=0.9, application/vnd.api+json;q=0.8, application/vnd.siren+json;q=0.8, application/vnd.collection+json;q=0.8, application/json;q=0.7, text/html;q=0.6
Accept-Language: en-GB
Accept-Encoding: gzip, deflate, br
Referer: http://localhost:3000/
Content-Type: application/json
Origin: http://localhost:3000
Connection: keep-alive
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
Pragma: no-cache
Cache-Control: no-cache
Content-Length: 2

{}

So it was setting the Content-Type header to application/json and it was sending a body of {}. Just that because the OpenAPI spec wasn't expecting any request body then Prism was rejecting even that as invalid.

Set the contentType field to application/x-www-form-urlencoded and now the request looks like:

POST /users/auth0%7C6240a09cc665610070aa5bfb/verify_email HTTP/1.1
Host: localhost:4010
User-Agent: Ketting/7.5.1
Accept: application/prs.hal-forms+json;q=1.0, application/hal+json;q=0.9, application/vnd.api+json;q=0.8, application/vnd.siren+json;q=0.8, application/vnd.collection+json;q=0.8, application/json;q=0.7, text/html;q=0.6
Accept-Language: en-GB
Accept-Encoding: gzip, deflate, br
Referer: http://localhost:3000/
Content-Type: application/x-www-form-urlencoded
Origin: http://localhost:3000
Connection: keep-alive
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
Pragma: no-cache
Cache-Control: no-cache
Content-Length: 0

And for some reason Prism seems this as valid - presumably because the body is empty, even though it does have a Content-Type header.

evert commented 2 years ago

Sorry, for the confusion, it was in response to @mamund :

I suspect it's a rule in prism that blindly attempts to parse a JSON object body when the content-type: application/json header is passed during some HTTP methods (e.g. POST/PUT/PATCH) -- even if no body is included.

I think it's pretty good to expect for a tool to attempt to parse the body if there was a Content-Type: application/json

mamund commented 2 years ago

my view on this is that the Prism server has a bug. would be interested in how other frameworks handle the same use case.

FWIW, the HAL-FORMS spec uses SHOULD, not MUST. that means client apps can choose to not send a content-type header with a body-less unssafe request.

it might help to update the spec (here: https://rwcbook.github.io/hal-forms/#_code_contenttype_code) to say that,

if there is no body sent with the request (for some set of HTTP methods?) then the client application MAY send no content-type header.

or some better wordsmitthing thereof.

the remainder is up to client and server implementors to sort out.

mamund commented 2 years ago

@evert :

I think it's pretty good to expect for a tool to attempt to parse the body if there was a Content-Type: application/json

agreed. but it might not be a good idea to reject the request. for example, application/x-forms-urlencoded is an example of what i think is the proper response.

and, as @sazzer points out,

It was sending a body though

that the body was {} and Prism is rejecting the request w/ 415. so it seems that Prism is getting a body that matches the stated content-type and still rejecting the request. that seems buggy.

evert commented 2 years ago

I'd strongly suggest coding 1 specific behavior for this, otherwise it will never be possible to create a generic client that works with generic servers without out-of-band configuration.

So if there's a desire to support POST request with no body/content-type, better make that explicit. The application/x-www-form-urlencoded may also be good enough though, but if you explicitly want another 'empty body' format, maybe it can be made explicit.

sazzer commented 2 years ago

Prism is rejecting the request not because the request is invalid, but because it's invalid with regards to the OpenAPI spec.

The problem at that end is that the OpenAPI spec says that this request shouldn't have a body but the client was sending one.

On Sun, 2 Oct 2022, 20:52 Evert Pot, @.***> wrote:

I'd strongly suggest coding 1 specific behavior for this, otherwise it will never be possible to create a generic client that works with generic servers without out-of-band configuration

— Reply to this email directly, view it on GitHub https://github.com/badgateway/ketting/issues/453#issuecomment-1264719862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQEGFDZIJ6E7CT7QVOYIDWBHRYPANCNFSM6AAAAAAQ23CFAE . You are receiving this because you were mentioned.Message ID: @.***>

mamund commented 2 years ago

@sazzer

good point.