Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Error exposing internals #14810

Closed
Nek- opened this issue Feb 15, 2023 · 1 comment
Closed

API Error exposing internals #14810

Nek- opened this issue Feb 15, 2023 · 1 comment
Assignees
Labels
API APIs related issues and PRs. Bug Confirmed bugs or bugfixes.

Comments

@Nek-
Copy link
Contributor

Nek- commented Feb 15, 2023

Sylius version affected: 1.13

Description
I wrongly formatted the following http request and ended with an error related to internals. I think such information should not be exposed WDYT?

Steps to reproduce

$ http POST https://master.demo.sylius.com/api/v2/shop/orders/2jHUl3N19G/items accept:application/ld+json Content-
Type:application/ld+json --raw='{"paymentMethod":"/api/v2/shop/payment-methods/bank_transfer"}'

HTTP/1.1 400 Bad Request
Cache-Control: no-cache, private
Connection: keep-alive
Content-Type: application/ld+json; charset=utf-8
Date: Wed, 15 Feb 2023 10:05:35 GMT
Link: <https://master.demo.sylius.com/api/v2/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"
Server: nginx/1.18.0 (Ubuntu)
Transfer-Encoding: chunked
X-Content-Type-Options: nosniff
X-Frame-Options: sameorigin

{
    "code": 400,
    "message": "Cannot create an instance of \"Sylius\\Bundle\\ApiBundle\\Command\\Cart\\AddItemToCart\" from serialized data because its constructor requires parameter \"productVariantCode\" to be present."
}
@NoResponseMate NoResponseMate added API APIs related issues and PRs. Bug Confirmed bugs or bugfixes. labels Jan 26, 2024
@NoResponseMate
Copy link
Contributor

Hi @Nek-

Agreed, leaking internals is a problem.
The class part should have been resolved by #15568

ATM for the same request you'll get:

{
  "code": 400,
  "message": "Request does not have the following required fields specified: productVariantCode."
}

which is still not perfect since productVariantCode is a constructor argument/property name instead of the name set in serialization, should've been productVariant instead and trying to use the former will just reiterate the same error so you'll loop with no end in sight... and at this moment of writing, I just went there and fixed it, lol.

Should be fine after #15775 gets merged and I'll close this once that happens.

Cheers 🍻

GSadee added a commit that referenced this issue Jan 29, 2024
… (NoResponseMate)

This PR was merged into the 1.13 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.13 |
| Bug fix?        | yes                                                       |
| New feature?    | no                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no |
| Related tickets | related #14810, #15568, #15615                     |
| License         | MIT                                                          |

Commits
-------

b025de3 [Maintenance][API] Normalize command missing field name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

No branches or pull requests

2 participants