Do we have any number regarding the performance penalty introduced by this check? As this class is often used to create web APId, we should be careful to not introduce a way to amplify DOS attacks.

Don't we already have this problem when signing in, which is very expensive in Symfony because of password encoding? I wouldn't be too harsh on that point here.

In prod, you'll likely set up some rate limiting for the login endpoint. Having a way to easily consume many resources on every endpoint is a different story.

Also, this feature is only useful during development. In production, it should be disabled as clients should never send invalid JSON anyway, and if they do it's easy to debug using client-side tools (most API development tools, such as Hoppscotch or Postman, include a JSON linter for instance).

I would at least make this feature opt-in through a context flag.

Problem I got inspired by is precisely Postman showing no issues with payload and PHP's json_decode refusing it (because of some invisible leading characters). And frontend dev was connecting to Prod API there. But I guess I could have reported issue to postman instead. Let's close here, I guess it's not too hard to decorate this class anyways.

To clarify I'm not against merging this feature as long as it's opt-in!

Addressed

Just a thought: Can/should we have FrameworkBundle enable this feature if the jsonlint packe is available and debug mode is on?

Alternatively, we could also do this in a recipe.

Just a thought: Can/should we have FrameworkBundle enable this feature if the jsonlint packe is available and debug mode is on?

Alternatively, we could also do this in a recipe.

@ostrolucky Do you want to implement that in that PR or in a future one?

Also, this feature is only useful during development. In production, it should be disabled as clients should never send invalid JSON anyway

Famous last words 😉

In my experience with consumer facing APIs you get quite often amateur level devs integrating things, sometimes writing json by hand, and for them having an API that returns helpful errors can save a bunch of time.

Just a thought: Can/should we have FrameworkBundle enable this feature if the jsonlint packe is available and debug mode is on?
Alternatively, we could also do this in a recipe.

@ostrolucky Do you want to implement that in that PR or in a future one?

Let's do it in next PR. Do you prefer recipe, or frameworkbundle doing this automatically on kernel.debug + when lib is installed?

Just a thought: Can/should we have FrameworkBundle enable this feature if the jsonlint packe is available and debug mode is on?
Alternatively, we could also do this in a recipe.

@ostrolucky Do you want to implement that in that PR or in a future one?

Let's do it in next PR. Do you prefer recipe, or frameworkbundle doing this automatically on kernel.debug + when lib is installed?

I think I would prefer a framework bundle integration.

Thank you@ostrolucky.

I'm sorry for being so late to this party but... would it be a bad idea to trigger seld/jsonlint whenever we actually encounter a syntax error? We don't need to lint everything, just (some of) the failures. All the other suggested changes can still stick (opt-in, when package is available etc). We can use it without taking a (severe) performance hit (we can have our cake and eat it too).

I've implemented my PR originally like that. It would trigger no matter the dev/prod environment. But I've changed it after the feedback provided, please check the conversation that we had here.

Yeah, I'm sorry, I've read the conversation, but I didn't read the code. Thanks for pointing it out and for the very useful feature!