cyberbotics / webots

Webots Robot Simulator
https://cyberbotics.com
Apache License 2.0
3.2k stars 1.68k forks source link

Long non-ASCII robot name crashes the controller. #5452

Closed amrytov closed 1 day ago

amrytov commented 1 year ago

Describe the Bug

See the subj.

Steps to Reproduce

  1. Create an empty world
  2. Put a Robot node in it. No child nodes are needed.
  3. Leave the <generic> controller as is.
  4. Name the robot with any long non-ascii name ("ЭтоДлинноеИмяДляРобота")
  5. Run the simulation

** What happens:

I've just seen each of the following, in different combinations:

Expected behavior

Either accept the name as valid and then work correctly, or report that the name is too long or contains illegal characters and not accept it.

Suggested possible solution(s):

(a) Check validity of the string assigned to the 'name' field (don't allow such names) (b) don't use robot name directly to create pipe names or whatever, instead create some fixed-length harmless hash string from them, so that any strings can be safely used.

Screenshots

2022-11-09 21_49_30-D__amrytov_documents_NTO_Space_worlds_TestLongNames wbt (NTO_Space) - Webots R20 2022-11-09 21_48_34-D__amrytov_documents_NTO_Space_worlds_TestLongNames wbt (NTO_Space) - Webots R20 2022-11-09 21_48_15-D__amrytov_documents_NTO_Space_worlds_TestLongNames wbt (NTO_Space) - Webots R20

System

brettle commented 3 weeks ago

I just ran into this bug when running the test suite on a Mac where my username was 11 characters long. The servername passed to QLocalServer (on Unix) has the form "/tmp/webots/username/1234/ipc/encodedRobotNname/intern" which is 29+usernameLength+encodedRobotNameLength. PR #5959 hashes encodedRobotNameLengths > 70 chars, but since the total length needs to be at most 104 chars (not 106 chars), that means the problem will still occur for users with usernames longer than (104-70-29) 5 characters. Since we have a total of 104-29=75 chars to work with, perhaps if either the username or the encodedRobotName is longer than 36 characters, it should get replaced with the first 36 characters of its hex hash.

brettle commented 3 weeks ago

It's actually a bit more complicated than I described, because in a Snap environment the prefix include $(HOME) which can be arbitrarily large. I'll be submitting a fix shortly. It will leave the prefix portion (everything through "ipc/" unchanged and just (if necessary) use a hash of the encodedRobotName that has been truncated as needed to make the resulting servername string fit on even the most restrictive system (which the Linux man page says means keeping it under 92 chars).

brettle commented 1 day ago

Fixed by PR #6635.