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

4KiB hard-coded buffer limits #51

Closed
mxjeff opened this issue Mar 5, 2020 · 2 comments
Closed

4KiB hard-coded buffer limits #51

mxjeff opened this issue Mar 5, 2020 · 2 comments

Comments

@mxjeff
Copy link

mxjeff commented Mar 5, 2020

Here is a follow-up of debian bug report #953110 :


Reproducer:

Uses mpc to send a 'find' request, containing an invalid search expression. The expected result is an error message about the invalid search expression.

working:

$ strace -s 40 -e trace=sendto,recvfrom mpc find '('$(for i in $(seq 1 4086); do echo -n "+"; done)')'
 recvfrom(3, "OK MPD 0.21.4\n", 4096, MSG_DONTWAIT, NULL, NULL) = 14
 sendto(3, "find \"(+++++++++++++++++++++++++++++++++"..., 4096, MSG_DONTWAIT, NULL, 0) = 4096
 recvfrom(3, "ACK [2@0] {find} Word expected\n", 4096, MSG_DONTWAIT, NULL, NULL) = 31
 mpd error: Word expected
 +++ exited with 1 +++

Note the request size on line 3: 4096 bytes.

broken:

The only difference is one more '+' in the expression, which would result in a request of 4097 bytes.

 $ strace -s 40 -e trace=sendto,recvfrom mpc find '('$(for i in $(seq 1 4087); do echo -n "+"; done)')'
 recvfrom(3, "OK MPD 0.21.4\n", 4096, MSG_DONTWAIT, NULL, NULL) = 14
 mpd error: Timeout
 +++ exited with 1 +++

Instead of error about the invalid request, a timeout occurs after 30 seconds because the request is not really sent (note the missing sendto() call).

Ideally, libmpdclient should support requests of arbitrary size (eventually reaching the server limit), but if that is not feasible, at least a proper error reporting would be nice.


src: https://bugs.debian.org/953110

@cataldor
Copy link
Contributor

Hello @mxjeff
thank you for the detailed report
i'm working on a solution for this
(sorry for the long delay)

cataldor added a commit to cataldor/libmpdclient that referenced this issue Mar 27, 2020
Allow the internal buffer of libmpdclient to grow dynamically in size
as needed by mpd_{async/sync}_send_command.
Previously, the buffer was limited to 4KiB.

Address issue reported downstream by Debian[1] and upstream by kaliko
(@mxjeff)[2]
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=953110
[2] MusicPlayerDaemon#51
cataldor added a commit to cataldor/libmpdclient that referenced this issue Mar 27, 2020
Identify messages that are over the 4KiB buffer limit

Address issue reported downstream by Debian[1] and upstream by kaliko
(@mxjeff)[2]
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=953110
[2] MusicPlayerDaemon#51
MaxKellermann pushed a commit that referenced this issue Jul 3, 2020
Identify messages that are over the 4KiB buffer limit

Address issue reported downstream by Debian[1] and upstream by kaliko
(@mxjeff)[2]
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=953110
[2] #51
@MaxKellermann
Copy link
Member

c383bbb

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

No branches or pull requests

3 participants