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

Fixed error where valueobject has eaten too much #229

Merged
merged 1 commit into from Jun 28, 2023

Conversation

mrbig
Copy link
Contributor

@mrbig mrbig commented Oct 5, 2022

This fixes a bug, when valueobject is an empty element like In this situation the deserializer did read one more, thus ending up a level higher than required.

I have also attached a unit test to demonstrate the problem. With the original code this xml did throw an error.

@evert
Copy link
Member

evert commented Oct 6, 2022

Pretty wild that people are still finding bugs, but LGTM!

@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #229 (f5073dc) into master (97ddaf2) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #229   +/-   ##
=========================================
  Coverage     96.90%   96.90%           
  Complexity      116      116           
=========================================
  Files            13       13           
  Lines           485      485           
=========================================
  Hits            470      470           
  Misses           15       15           
Impacted Files Coverage Δ
lib/Deserializer/functions.php 89.69% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mrbig
Copy link
Contributor Author

mrbig commented Oct 6, 2022

Pretty wild that people are still finding bugs, but LGTM!

agree with that, I was surprised, that this haven't been found yet.

Thanks for the great software!

@phil-davis phil-davis self-assigned this Jun 28, 2023
@phil-davis
Copy link
Contributor

This has been here for a while!
I will have a look and see if I can make bugfix patch releases for major versions 2, 3 and 4.

This fixes a bug, when valueobject is an empty element like <foo></foo>
In this situation the deserializer did read one more, thus ending up a level
higher than required
@phil-davis phil-davis merged commit 7bc0644 into sabre-io:master Jun 28, 2023
6 checks passed
@phil-davis
Copy link
Contributor

phil-davis commented Jun 28, 2023

Backport to v3 branch is in PR #241
Backport to 2.1 branch is in PR #242 (actually that is not needed)
Backport to 2.2 branch is in PR #243 (and I will make a next patch release of the 2.2 series)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants