Skip to content

Promote warnings to exceptions in ext/soap and ext/xmlwriter - #5998

Closed
kocsismate wants to merge 2 commits into
php:masterfrom
kocsismate:soap-warning
Closed

Promote warnings to exceptions in ext/soap and ext/xmlwriter#5998
kocsismate wants to merge 2 commits into
php:masterfrom
kocsismate:soap-warning

Conversation

@kocsismate

Copy link
Copy Markdown
Member

No description provided.

Comment threadext/soap/soap.c Outdated

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I see, this PR will eliminate a few cases when a constructor returned null... :)

@kocsismate
kocsismateforce-pushed the soap-warning branch 6 times, most recently from df10d30 to 955f289CompareAugust 17, 2020 07:15
@phpphp deleted a comment Aug 24, 2020
Comment threadext/soap/soap.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to wonder wtf UNLIMATERECEIVER is supposed to mean...

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D Do you also suppose it's "ultimate receiver"? As far as I see, Google shows quite some hits for "soap ultimate receiver" :D

Comment threadext/soap/soap.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is a bit misleading, in that it implies that it accepts arbitrary callbacks (not just global functions).

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to "must be a valid function name", if that's ok for you

Comment threadext/soap/soap.c Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be one more else below to handle a completely invalid type?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I missed that, thanks!

Comment threadext/soap/soap.c
php_error_docref(NULL, E_WARNING, "Tried to add a non existent function '%s'", Z_STRVAL_P(tmp_function));
zend_type_error("SoapServer::addFunction(): Function \"%s\" not found", Z_STRVAL_P(tmp_function));
RETURN_THROWS();
return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant return

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed when applying!

@kocsismate
kocsismate deleted the soap-warning branch September 1, 2020 12:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@kocsismate@nikic