-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[mysql] Fix ArrayIndexOutOfBoundsException in mysql binlog read phase #911
Conversation
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.
Thanks for your great work!
...mysql-cdc/src/test/java/com/github/shyiko/mysql/binlog/io/BufferedSocketInputStreamTest.java
Outdated
Show resolved
Hide resolved
...tor-mysql-cdc/src/main/java/com/github/shyiko/mysql/binlog/io/BufferedSocketInputStream.java
Outdated
Show resolved
Hide resolved
...tor-mysql-cdc/src/main/java/com/github/shyiko/mysql/binlog/io/BufferedSocketInputStream.java
Outdated
Show resolved
Hide resolved
offset = 0; | ||
limit = in.read(buffer, 0, buffer.length); | ||
if (limit == -1) { | ||
return limit; |
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.
-1 in the InputStream means EOF, which will cause the ByteArrayInputStream#fill
to throw the EOF Exception. BinaryLogClient
may still need to exit in its method listenForEventPackets
.
I just curious in which situation we will receive -1 here? Actually the inputstream is from the Socket. I think it should never reach end ? Can we return 0 rather than -1 to represents retry here?
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 think we should return -1 here to follow the mehod java doc, reach the EOF of inpustream is expected, see more in NamedPipeSocketFactory.this.new RandomAccessFileInputStream
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.
You are right. It should return -1 and the invoker will take over and reconnect.
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.
LGTM
Try to fix #460