feat(common): serialize to JSON if explicitly set in Content-Type and… #31991
Conversation
… not already serialized RFC 8259 and ECMA-404 define a simple string, boolean and null as valid JSON. If Content-Type has been explicitly set to 'application/json' the HttpClient should determine, if the body has already been serialized and if not, serialize it. Not breaking. Relates to angular#31973, angular#31974
…h whitespace A serialized JSON string can start with whitespaces by definition. Allows this case to pass the checks and not be serialized again. Combined with the other commits of PR angular#31991 this is not a breaking change.
dasois
force-pushed
the
feat_request-json-smart-stringify
branch
from
27a5d8c
to
b7b83f4
Compare
…y starting with whitespace A serialized JSON string can start with whitespaces by definition. Allows this case to pass the checks and not be serialized again. Combined with the other commits of PR angular#31991 this is not a breaking change.
dasois
force-pushed
the
feat_request-json-smart-stringify
branch
from
b7b83f4
to
7e2570a
Compare
…y starting with whitespace A serialized JSON string can start with whitespaces by definition. Allows this case to pass the checks and not be serialized again. Combined with the other commits of PR angular#31991 this is not a breaking change.
dasois
force-pushed
the
feat_request-json-smart-stringify
branch
from
7e2570a
to
7b41d67
Compare
…y starting with whitespace A serialized JSON string can start with whitespaces by definition. Allows this case to pass the checks and not be serialized again. Combined with the other commits of PR angular#31991 this is not a breaking change.
…y starting with whitespace A serialized JSON string can start with whitespaces by definition. Allows this case to pass the checks and not be serialized again. Combined with the other commits of PR angular#31991 this is not a breaking change.
dasois
force-pushed
the
feat_request-json-smart-stringify
branch
from
7b41d67
to
7649d4b
Compare
|
codefresh-bazel build seems to be broken, I don't think my change introduced this. |
|
What’s the status on this issue? Will this be merged anytime soon? |
|
Any news when this will be merged? |
|
@IgorMinar @AndrewKushnir Can I do anything to get this merged? |
petebacondarwin
added
action: review
|
This PR has the potential to be a breaking change, but it's definitely a valid concern. We're going to close this PR for now, but we're discussing internally a good solution to this problem. There is no ETA at this moment. |
|
@jessicajaniuk could you share some insight into why this is a potentially breaking change? Maybe it could be addressed, instead of just closing the issue? |
|
@fpaaske Sorry for the delay on this response. The reason it's breaking is because HttpClient is easily one of the most used services with a lot of people already connecting to endpoints that have the content-type set. All of these applications have code that is currently expecting strings and then convert it to a useable type afterwards. If we change this behavior, it literally breaks everyone in that situation. The possibility of a migration to address the change wouldn't be possible either due to the vastness of the usage. HttpClient, itself, has been neglected for quite a while. It lacks strong typing, has hard dependencies on RxJS, and has this issue of serialization. Rather than updating the current implementation and risk breaking so many, we're having early conversations about potentially building a new and updated HttpClient that addresses all of these issues. It's still very early in that conversation, but there will likely be a proposal for it in the coming months. |
|
Thanks for your explanation, hopefully there will be a good solution in the future! |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
… not already serialized
RFC 8259 and ECMA-404 define a simple string, boolean and null as valid JSON.
If Content-Type has been explicitly set to 'application/json' the HttpClient should
determine if the body has already been serialized and if not, serialize it.
Not breaking.
Relates to #31973, #31974
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently you are able to e.g. provide a string, that is not a serialized JSON and want to send it to a Server that expects 'application/json' Content-Type. The HttpClient would not serialize that string and thus send an invalid JSON string and parsing will fail on the server side.
Even though HttpClient knows it needs to send JSON, it doesn't make sure that the data already has been serialized. It just assumes that this is the case for a string.
Relating Issue Number: #31973
What is the new behavior?
If Content-Type is explicitly set to 'application/json', the HttpClient will check the body, if it is a string, if it is already parsed JSON and otherwise parse it.
Does this PR introduce a breaking change?
Other Notes
We already tried to solve this issue in #31974, but the discussions lead to a non-breaking solution being a better choice here, which was the motivation for this PR.