Skip to content

Fix GH-10521: ftp_get/ftp_nb_get resumepos offset is maximum 10GB #10525

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.

Sign up for GitHub

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

Jump to bottom
Closed
ndossche wants to merge 1 commit into from
Closed

Fix GH-10521: ftp_get/ftp_nb_get resumepos offset is maximum 10GB #10525

ndossche wants to merge 1 commit into from

Conversation

@ndossche
Copy link
Member

@ndossche ndossche commented Feb 6, 2023 •
edited
Loading

Fixes GH-10521

The char arrays were too small for a long on 64-bit systems, which resulted in cutting off the string at the end with a NUL byte. Use a size of MAX_LENGTH_OF_LONG to fix this issue instead of a fixed size of 11 chars.

@devnexen devnexen requested a review from cmb69 February 7, 2023 08:06
@mvorisek
Copy link
Contributor

mvorisek commented Feb 7, 2023

test is not passing on MacOS

@ndossche
Fix phpGH-10521: ftp_get/ftp_nb_get resumepos offset is maximum 10GB
879987f 
The char arrays were too small for a long on 64-bit systems, which
resulted in cutting off the string at the end with a NUL byte. Use a
size of MAX_LENGTH_OF_LONG to fix this issue instead of a fixed size
of 11 chars.
@ndossche
Copy link
Member Author

ndossche commented Feb 7, 2023

test is not passing on MacOS

Thanks for letting me know, should be fixed now.

devnexen
devnexen approved these changes Feb 8, 2023
View reviewed changes
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

LGTM but still waiting @cmb69 for this one :)

@ndossche ndossche mentioned this pull request Feb 10, 2023
Closed
@ndossche
Copy link
Member Author

ndossche commented Mar 15, 2023

Christoph seems quite busy lately, shall we ask someone else for review too or is one reviewer good enough? :)

@ndossche ndossche requested a review from iluuu1994 March 20, 2023 22:12
@iluuu1994
Copy link
Member

iluuu1994 commented Mar 20, 2023

Thanks 🙂

@ndossche ndossche self-assigned this Mar 20, 2023
@ndossche ndossche closed this in 3014182 Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iluuu1994 iluuu1994 iluuu1994 approved these changes

@devnexen devnexen devnexen approved these changes

@cmb69 cmb69 Awaiting requested review from cmb69

Assignees

@ndossche ndossche

Labels

Extension: ftp

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ndossche @mvorisek @iluuu1994 @devnexen