fix(http): prevent headers from throwing an error when initializing numerical values by danilobassi8 · Pull Request #49379 · angular/angular

danilobassi8 · 2023-03-09T13:37:29Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Some libraries could use numbers in headers, but Angular is throwing an error when initializing Headers with numerical values

Issue Number: #49353

What is the new behavior?

Angular won't throw an error if there's some number in HTTP Headers. It will just cast those values into string.

Does this PR introduce a breaking change?

Yes
No

Other information

google-cla · 2023-03-09T13:37:34Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

jessicajaniuk

LGTM

Thanks for this!

packages/common/http/src/headers.ts

JeanMeche · 2023-03-21T19:03:03Z

You will need to update the golden files with yarn bazel run //packages/common:common_api.accept to pass the circleci test !

…umerical values Some libraries could use numbers in headers. this fix prevents Angular from throwing an error by casting those numerical values into strings. Fixes angular#49353

danilobassi8 · 2023-03-22T13:30:21Z

You will need to update the golden files with yarn bazel run //packages/common:common_api.accept to pass the circleci test !

@JeanMeche Done! let me know if there's something more to be done. Its my first time opening a PR to this repo!

alxhub

Reviewed-for: public-api

danilobassi8 · 2023-04-13T12:27:11Z

Reviewed-for: public-api

Let me know if there's something more I can do to clear the cleanup tag. I don't really know your usual workflow, so I'm just waiting for instructions

ngbot · 2023-04-13T13:37:29Z

I see that you just added the action: merge label, but the following checks are still failing:
status "pullapprove" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

jessicajaniuk · 2023-04-13T13:38:07Z

Caretaker: this is safe to merge

jessicajaniuk · 2023-04-13T14:00:19Z

This PR was merged into the repository by commit ab5e2d9.

…umerical values (#49379) Some libraries could use numbers in headers. this fix prevents Angular from throwing an error by casting those numerical values into strings. Fixes #49353 PR Close #49379

danilobassi8 · 2023-04-17T14:24:56Z

This PR was merged into the repository by commit ab5e2d9.

Great! What is the min version that I should use to see this changes on my project?? (If its already published)

JeanMeche · 2023-04-17T14:27:08Z

You can see that the PR was tagged as target: Patch so it will land in the next patch. (Likely this week I think)

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.2.6/15.2.8) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.2.6/15.2.8) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.2.6/15.2.8) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.2.6/15.2.8) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.2.6/15.2.8) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.2.6/15.2.8) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.2.6/15.2.8) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`15.2.6` -> `15.2.8`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.2.6/15.2.8) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v15.2.8`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1528-2023-04-19) [Compare Source](angular/angular@15.2.7...15.2.8) ##### core | Commit | Type | Description | | -- | -- | -- | | [2fff8fadbe](angular/angular@2fff8fa) | fix | handle invalid classes in class array bindings ([#49924](angular/angular#49924)) | ##### http | Commit | Type | Description | | -- | -- | -- | | [05a0225deb](angular/angular@05a0225) | fix | prevent headers from throwing an error when initializing numerical values ([#49379](angular/angular#49379)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [09a42d988e](angular/angular@09a42d9) | fix | canceledNavigationResolution: 'computed' with redirects to the current URL ([#49793](angular/angular#49793)) |  ### [`v15.2.7`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1527-2023-04-12) [Compare Source](angular/angular@15.2.6...15.2.7) ##### compiler | Commit | Type | Description | | -- | -- | -- | | [b0c1a90f55](angular/angular@b0c1a90) | fix | Produce diagnositc if directive used in host binding is not exported ([#49792](angular/angular#49792)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [a40529af2e](angular/angular@a40529a) | fix | Catch FatalDiagnosticError during template type checking ([#49792](angular/angular#49792)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [702ec90110](angular/angular@702ec90) | fix | When using setInput, mark view dirty in same way as `markForCheck` ([#49747](angular/angular#49747)) | #### Special Thanks Alan Agius, Andrew Kushnir, Andrew Scott, Kristiyan Kostadinov, Matthieu Riegler and Nikola Kološnjaji  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1863 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2023-05-18T00:08:15Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

pullapprove bot requested a review from jessicajaniuk March 9, 2023 13:37

danilobassi8 force-pushed the fix-headers-assertion-for-ints branch from a09deaa to 6896ca6 Compare March 9, 2023 13:55

jessicajaniuk added the area: common/http Issues related to HTTP and HTTP Client label Mar 9, 2023

ngbot bot added this to the Backlog milestone Mar 9, 2023

jessicajaniuk approved these changes Mar 13, 2023

View reviewed changes

jessicajaniuk added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Mar 13, 2023

alxhub requested changes Mar 14, 2023

View reviewed changes

packages/common/http/src/headers.ts Outdated Show resolved Hide resolved

packages/common/http/src/headers.ts Outdated Show resolved Hide resolved

alxhub added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Mar 14, 2023

danilobassi8 force-pushed the fix-headers-assertion-for-ints branch from 6896ca6 to da6a4f4 Compare March 14, 2023 16:49

danilobassi8 requested a review from alxhub March 14, 2023 17:47

danilobassi8 force-pushed the fix-headers-assertion-for-ints branch 3 times, most recently from cb6c081 to f8e6c39 Compare March 20, 2023 14:51

danilobassi8 requested review from JeanMeche and removed request for alxhub March 21, 2023 19:01

fix(http): prevent headers from throwing an error when initializing n…

b7f88b1

…umerical values Some libraries could use numbers in headers. this fix prevents Angular from throwing an error by casting those numerical values into strings. Fixes angular#49353

danilobassi8 force-pushed the fix-headers-assertion-for-ints branch from f8e6c39 to b7f88b1 Compare March 21, 2023 19:10

danilobassi8 requested review from alxhub and jessicajaniuk and removed request for JeanMeche and alxhub March 23, 2023 16:11

jessicajaniuk approved these changes Apr 3, 2023

View reviewed changes

alxhub approved these changes Apr 3, 2023

View reviewed changes

jessicajaniuk removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 13, 2023

jessicajaniuk added the action: merge The PR is ready for merge by the caretaker label Apr 13, 2023

jessicajaniuk added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Apr 13, 2023

jessicajaniuk closed this in ab5e2d9 Apr 13, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 18, 2023

Conversation

danilobassi8 commented Mar 9, 2023

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

google-cla bot commented Mar 9, 2023

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JeanMeche commented Mar 21, 2023

Uh oh!

danilobassi8 commented Mar 22, 2023

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

danilobassi8 commented Apr 13, 2023

Uh oh!

ngbot bot commented Apr 13, 2023

Uh oh!

jessicajaniuk commented Apr 13, 2023

Uh oh!

jessicajaniuk commented Apr 13, 2023

Uh oh!

danilobassi8 commented Apr 17, 2023

Uh oh!

JeanMeche commented Apr 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angular-automatic-lock-bot bot commented May 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JeanMeche commented Apr 17, 2023 •

edited

Loading