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

[Backport 1.3] [1.x backport] Bump joi to v14 to avoid the possibility of prototype poisoning in a nested dependency #4345

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 4626066 from #4211.

…e poisoning in a nested dependency (#4211)

Backport PR
#3952

Signed-off-by: Miki <miki@amazon.com>
Co-authored-by: Miki <miki@amazon.com>
(cherry picked from commit 4626066)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
AMoo-Miki
AMoo-Miki previously approved these changes Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #4345 (6f96484) into 1.3 (b3c3fd7) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              1.3    #4345      +/-   ##
==========================================
+ Coverage   67.45%   67.50%   +0.04%     
==========================================
  Files        3044     3044              
  Lines       58692    58692              
  Branches     8902     8902              
==========================================
+ Hits        39591    39619      +28     
+ Misses      16947    16925      -22     
+ Partials     2154     2148       -6     
Flag Coverage Δ
Linux 67.45% <ø> (+<0.01%) ⬆️
Windows 67.45% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 files with indirect coverage changes

@AMoo-Miki
Copy link
Collaborator

I am not happy to see joi@13.x.x still there.

@joshuarrrr
Copy link
Member

This is a major version bump - I didn't see any analysis about how/why this is safe to backport to the 1.x line - can we provide that here?

manasvinibs
manasvinibs previously approved these changes Jun 22, 2023
Signed-off-by: Josh Romero <rmerqg@amazon.com>
@joshuarrrr joshuarrrr dismissed stale reviews from manasvinibs and AMoo-Miki via 37a79d5 June 22, 2023 20:08
@ashwin-pc
Copy link
Member

here are the breaking changes for going from 13 to 14 hapijs/joi#1615

  • stripUnknown: only one use and does not affect us
  • Stricter and safer parsing of numbers: Again should not affect us
  • Fixed output result of Joi.string().valid(...).insensitive(): Does not affect us
  • Nested paths on object.or/nand/and/xor/with/without(): We dont seem to be using these operations
  • Examples are now validated again and can be replaced: We dont use .example()
  • Lazy schemas are now less lazy by default : We dont use lazy schemas
  • array.min/max/length will be overridden by last call: We dont use array().min/max/length
  • object.pattern and string.regex now preserve most of the flags: None of our uses use any of the regex flags
  • Single year is now parsed correctly for ISO dates: We dont use this
  • Correctly apply labels to alternatives schemas: We dont use this
  • Error message override is also applied to individual errors: only affects the error details to be more consistent. Does not affect our uses of JOI.

Based on this i think this is safe to backport to 1.3

@ashwin-pc ashwin-pc self-assigned this Jun 28, 2023
ashwin-pc
ashwin-pc previously approved these changes Jun 28, 2023
AMoo-Miki
AMoo-Miki previously approved these changes Jun 28, 2023
Signed-off-by: Qingyang(Abby) Hu <abigailhu2000@gmail.com>
@abbyhu2000 abbyhu2000 dismissed stale reviews from AMoo-Miki and ashwin-pc via 6f96484 July 25, 2023 22:29
@joshuarrrr joshuarrrr merged commit a675b1c into 1.3 Jul 26, 2023
36 of 37 checks passed
@joshuarrrr joshuarrrr deleted the backport/backport-4211-to-1.3 branch July 26, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants