Skip to content

Conversation

@arshidkv12
Copy link

@arshidkv12 arshidkv12 requested a review from bukka as a code owner January 18, 2026 03:58
case SEEK_CUR:
if (offset < 0) {
if (ms->fpos < (size_t)(-offset)) {
if (ms->fpos < -(size_t)offset) {
Copy link
Member

Choose a reason for hiding this comment

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

The error goes away, but it's not really correct. The - effectively becomes a no-op. However, the intent here is to compare ms->fpos to a positive offset.

I'm not sure if we need this check at all? For positive offset, we're not checking for overflows and rely on eof checks when reading from the stream, so do we need to check for underflows for negative offset? It would simply wrap around the same way positive offsets do. Though I suppose removing this check is a subtle breaking change.

Copy link
Author

Choose a reason for hiding this comment

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

I changed it as before.

case SEEK_CUR:
  if (offset < 0) {
-  if (ms->fpos < -(size_t)offset) {
+  if (ms->fpos < (size_t)(-offset)) {

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, but my comment was referring to both cases. I suppose the fix is ok to avoid the undefined behavior, though it's probably not the optimal fix either.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. Adding the following code in the first part of the php_stream_memory_seek function, what do you think?

	if (offset == ZEND_LONG_MIN) {
		zend_argument_value_error(2, "must be greater than " ZEND_LONG_FMT, ZEND_LONG_MIN);
		return FAILURE;
	}  

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.

2 participants