-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Promote warnings to Error in IMAP #6164
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
Conversation
ce0c983
to
ce5e858
Compare
@@ -830,6 +835,17 @@ PHP_FUNCTION(imap_reopen) | |||
RETURN_THROWS(); | |||
} | |||
|
|||
if (options && ((options & ~(OP_READONLY | OP_ANONYMOUS | OP_HALFOPEN | OP_EXPUNGE | CL_EXPUNGE)) != 0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you determine which options are valid for which function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled them from the PHP.net documentation, as I would expect the options which are documented there to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question then is whether imap_reopen only lists part of the flags because the rest are not supported, or because only the imap_open docs were updated ^^
ce5e858
to
a6acaa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
*** Testing imap_fetch_overview() : usage variations *** | ||
Create a temporary mailbox and add 1 msgs | ||
.. mailbox '{%s}%s' created | ||
.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wildcard got lost.
*** Testing imap_fetchbody() : usage variations *** | ||
Create a temporary mailbox and add 1 msgs | ||
.. mailbox '{%s}%s' created | ||
.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wildcard got lost.
*** Testing imap_fetchheader() : usage variations *** | ||
Create a temporary mailbox and add 1 msgs | ||
.. mailbox '{%s}%s' created | ||
.. mailbox '{127.0.0.1:143/norsh}INBOX.phpttest' created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wildcard got lost.
@@ -830,6 +835,17 @@ PHP_FUNCTION(imap_reopen) | |||
RETURN_THROWS(); | |||
} | |||
|
|||
if (options && ((options & ~(OP_READONLY | OP_ANONYMOUS | OP_HALFOPEN | OP_EXPUNGE | CL_EXPUNGE)) != 0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question then is whether imap_reopen only lists part of the flags because the rest are not supported, or because only the imap_open docs were updated ^^
body = mail_fetchtext_full (imap_le_struct->imap_stream, msgno, &body_len, (argc == 3 ? flags : NIL)); | ||
PHP_IMAP_CHECK_MSGNO(msgindex, 2); | ||
|
||
/* TODO Shouldn't this pass msgindex??? */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh
PHP_IMAP_CHECK_MSGNO(msgno, 2); | ||
|
||
if (fromlength < 0 || fromlength > MAILTMPLEN) { | ||
RETURN_THROWS(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong way around :)
Discovered several potential bug and integer overflows.