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

Component/select: Before uppercasing $child->group, make sure it isn't null #573

Merged
merged 1 commit into from May 12, 2022
Merged

Component/select: Before uppercasing $child->group, make sure it isn't null #573

merged 1 commit into from May 12, 2022

Conversation

algernon
Copy link
Contributor

I recently set up Baïkal, and it works wonderfully with most clients I let at it. However, I ran into an issue when I tried using InfCloud pointed at it: my address book didn't display, because Baïkal returned a HTTP 500 to some request InfCloud was making.

I dug into the logs, and I found that the issue is this backtrace:

today at 1:58:13 AM2022/05/11 23:58:13 [error] 23#23: *1387 FastCGI sent in stderr: "PHP message: ErrorException: strtoupper(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/baikal/vendor/sabre/vobject/lib/Component.php:252
today at 1:58:13 AMStack trace:
today at 1:58:13 AM#0 [internal function]: Baikal\Framework::exception_error_handler()
today at 1:58:13 AM#1 /var/www/baikal/vendor/sabre/vobject/lib/Component.php(252): strtoupper()
today at 1:58:13 AM#2 /var/www/baikal/vendor/sabre/vobject/lib/VCardConverter.php(119): Sabre\VObject\Component->select()
today at 1:58:13 AM#3 /var/www/baikal/vendor/sabre/vobject/lib/VCardConverter.php(55): Sabre\VObject\VCardConverter->convertProperty()
today at 1:58:13 AM#4 /var/www/baikal/vendor/sabre/vobject/lib/Component/VCard.php(189): Sabre\VObject\VCardConverter->convert()
today at 1:58:13 AM#5 /var/www/baikal/vendor/sabre/dav/lib/CardDAV/Plugin.php(829): Sabre\VObject\Component\VCard->convert()
today at 1:58:13 AM#6 /var/www/baikal/vendor/sabre/dav/lib/CardDAV/Plugin.php(244): Sabre\CardDAV\Plugin->convertVCard()
today at 1:58:13 AM#7 /var/www/baikal/vendor/sabre/dav/lib/CardDAV/Plugin.php(189): Sabre\CardDAV\Plugin->addressbookMultiGetReport()

Naturally, I went looking, as this seemed easy to fix, or at least work around. I have no idea what request InfCloud is sending, mind you. I looked at it, it was a lot of XML, I ran away. So I looked at the Baïkal side instead, tracked down the place the error was coming from, and did as this patch does: before calling strtoupper($child->group), I added another check to see if $child->group is non-null.

Now I have my address book shown via InfCloud too, and everything else appears to work as well.

I'm not sure if its the right fix, but it gets the job done, so I figured I'll share.

…t null

Signed-off-by: Gergely Nagy <me@gergo.csillger.hu>
@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #573 (7c50120) into master (06feff3) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #573      +/-   ##
============================================
- Coverage     98.34%   98.34%   -0.01%     
- Complexity     1884     1885       +1     
============================================
  Files            71       71              
  Lines          4536     4529       -7     
============================================
- Hits           4461     4454       -7     
  Misses           75       75              
Impacted Files Coverage Δ
lib/Component.php 99.15% <100.00%> (ø)
lib/PHPUnitAssertions.php 91.66% <0.00%> (-0.34%) ⬇️
lib/Parser/Json.php 96.87% <0.00%> (-0.14%) ⬇️
lib/Parser/XML.php 97.45% <0.00%> (-0.05%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@DeepDiver1975 DeepDiver1975 merged commit a595790 into sabre-io:master May 12, 2022
@algernon algernon deleted the component-select-no-group-fix branch May 12, 2022 11:03
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

3 participants