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

Returned byte buffers through ByteBufferIterator from DefaultDataBuffer are misconfigured #30966

Closed
access-control-rtfm opened this issue Jul 31, 2023 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: bug A general bug

Comments

@access-control-rtfm
Copy link

access-control-rtfm commented Jul 31, 2023

Affects: 6.0.11


The DefaultDataBuffer class has follow implementations of the methods readableByteBuffers and writableByteBuffers of DataBuffer interface:

@Override
public DataBuffer.ByteBufferIterator readableByteBuffers() {
  ByteBuffer readOnly = this.byteBuffer.asReadOnlyBuffer();
  readOnly.clear().position(this.readPosition).limit(this.writePosition - this.readPosition);
  return new ByteBufferIterator(readOnly);
}
@Override
public DataBuffer.ByteBufferIterator writableByteBuffers() {
  ByteBuffer duplicate = this.byteBuffer.duplicate();
  duplicate.clear().position(this.writePosition).limit(this.capacity - this.writePosition);
  return new ByteBufferIterator(duplicate);
}

The limit on readOnly in readableByteBuffers should be set to this.writePosition, because if the condition (this.writePosition - this.readPosition) <= this.readPosition is satisfied, the limit of the returned byte buffer will be equal to its position, so the byte buffer will be returned, which no single byte could be read from. In this case the position can be set to a lower index than this.readPosition, which can lead that some buffer data will be read again, after limit increased.

The limit on duplicate in writableByteBuffers should be set to this.capacity, because if the condition (this.capacity - this.writePosition) <= this.writePosition is satisfied, the limit of the returned byte buffer will be equal to its position, so the byte buffer will be returned, which no single byte could be written into. In this case the position can be set to a lower index than this.writePosition, which can lead, that some buffer data will be overwritten, after limit increased.

So how it should be:

@Override
public DataBuffer.ByteBufferIterator readableByteBuffers() {
  ByteBuffer readOnly = this.byteBuffer.asReadOnlyBuffer().limit(this.writePosition).position(this.readPosition);
  return new ByteBufferIterator(readOnly);
}
@Override
public DataBuffer.ByteBufferIterator writableByteBuffers() {
  ByteBuffer duplicate = this.byteBuffer.duplicate().limit(this.capacity).position(this.writePosition);
  return new ByteBufferIterator(duplicate);
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 31, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Aug 2, 2023
@sbrannen
Copy link
Member

sbrannen commented Aug 2, 2023

Thanks for reporting the issue. We'll look into it.

@injae-kim
Copy link
Contributor

0c22866

	@Override
	public DataBuffer.ByteBufferIterator readableByteBuffers() {
		ByteBuffer readOnly = this.byteBuffer
                    .slice(this.readPosition, readableByteCount()) // use slice(start, len)
                    .asReadOnlyBuffer();
		return new ByteBufferIterator(readOnly);
	}

I think this issue is fixed by above commit, so we can close this issue too~! cc. @poutsma

@poutsma poutsma added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 11, 2024
@poutsma
Copy link
Contributor

poutsma commented Jan 11, 2024

Thanks @injae-kim, closing as duplicate of #31873.

@poutsma poutsma closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
@poutsma poutsma added the status: duplicate A duplicate of another issue label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants