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

Fix use of ST_MSGN as sequence method #356

Merged
merged 1 commit into from
Mar 1, 2023
Merged

Conversation

gioid
Copy link
Contributor

@gioid gioid commented Jan 10, 2023

When we upgraded from version 2.7.2 to 4.1.0 of this library we noticed bugs and sneaky behaviours using the sequence method ST_MSGN.
We think that these two issues suffered the same behaviours that we noticed:

We checked the differences among old version and new version. We found that the main issue regards the use of the $uid variable in the "fetch" method of the ImapProtocol class. The old version used that variable as a boolean while the new code treats it sometimes as bool and sometimes as int|string.

This pull request restored the old behaviour and with these changes we can see our code started work again.
Can you please check it and understand if you want to merged it in the master branch?

@gioid gioid marked this pull request as ready for review January 10, 2023 11:38
@didi1357
Copy link
Contributor

+1

Did not run into the bug, but saw it in code when looking for how to ensure that UIDs are used. Fortunately for me, they are default in this library in a lot of places and PHP seems to treat the newly introduced constant IMAP::ST_UID as true in many places. However, the current code for sure will not work properly with the message numbers constant.

@gioid seems to have found all occurences of the problem and fixed the comparisons. I approve and would merge his changes. However, I have no write access. Just my two cents as info for @Webklex. :)

@Webklex
Copy link
Owner

Webklex commented Mar 1, 2023

Hi @gioid ,
many thanks for your pull request as well as time and effort to look for all occurrences.

Best regards,

@Webklex Webklex merged commit e108c27 into Webklex:master Mar 1, 2023
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.

3 participants