fix exceptions caused by gwt 2.8.2 checking charAt bounds #4451
Conversation
…ing.charAt - fixed the two I was able to repro, plus made some preventative changes in some other places where I couldn't determine safety of existing code
|
LGTM but I would (marginally) prefer a routine |
Might still do that if we hit a bunch more of these and it becomes tedious to fix them. I prefer we try and adhere to the Java behavior since that's what we're kinda coding in and it seems sloppy of us to be relying on ability to go OOB accessing strings and potentially confounds static code analysis tools. Of course the JavaScript charAt is perfectly happy to do that and returns an empty string, I believe. The Gwt 2.8.1 implementation of Java charAt, however, was returning NaN in one case I stepped through. |
JavaScript exception related to charAt; don't know exact repro, taking opportunity to fix that call stack and defensively change a few other instances of charAt to use safer StringUtil.charAt. For more context: * fix exceptions caused by gwt 2.8.2 checking charAt bounds #4451 * more fixes for new behavior of String.charAt in GWT 2.8.2 #4489 * fix exception when typing Shift+Tab on whitespace-only editor line #5036 * javascript exceptions being thrown by charAt in cpp completion code #5052 Callstack: ``` 15 Feb 2020 00:03:26 [rserver] ERROR CLIENT EXCEPTION (rsession-gary): Index: -1, Size: 226; rstudio-0.js@10#639::Throwable.createError rstudio-0.js@48#703::Throwable.initializeBackingError rstudio-0.js@8#535::Throwable.Throwable rstudio-0.js@18#808::Exception.Exception rstudio-0.js@18#862::RuntimeException.RuntimeException rstudio-0.js@25#3620::IndexOutOfBoundsException.IndexOutOfBoundsException rstudio-0.js@34#5360::StringIndexOutOfBoundsException rstudio-0.js@21#5794::checkCriticalStringElementIndex rstudio-0.js@5#6025::checkStringElementIndex rstudio-0.js@3#4059::charAt rstudio-0.js@10#4633::charAt_I_C__devirtual$ rstudio-0.js@39#356896::CompletionManagerBase.canAutoPopup rstudio-0.js@14#357339::CompletionManagerBase.previewKeyPress rstudio-0.js@61#360873::MarkdownCompletionManager.previewKeyPress rstudio-0.js@23#359555::DelegatingCompletionManager.previewKeyPress rstudio-0.js@43#464919::AceKeyboardPreviewer$1.previewKeyPress rstudio-0.js@24#464884::AceKeyboardPreviewer.onTextInput rstudio-0.js@21#464852::<anonymous> rstudio-0.js@28#16334::apply rstudio-0.js@16#16393::entry0 rstudio-0.js@14#16372::handleKeyboard http://localhost:9876/rstudio/7D3FB0FCC17B7BB289C4D681EC4DE752.cache.js@43#42254::callKeyboardHandlers http://localhost:9876/rstudio/7D3FB0FCC17B7BB289C4D681EC4DE752.cache.js@14#42290::onTextInput http://localhost:9876/rstudio/7D3FB0FCC17B7BB289C4D681EC4DE752.cache.js@36#49554::onTextInput http://localhost:9876/rstudio/7D3FB0FCC17B7BB289C4D681EC4DE752.cache.js@22#40398::sendText http://localhost:9876/rstudio/7D3FB0FCC17B7BB289C4D681EC4DE752.cache.js@24#40419::onInput Client-ID: 3049a086-a908-4534-a87e-4dcaca02d7f7 User-Agent: Mozilla/5.0 (Macintosh Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.106 Safari/537.36 ```
One of the fixes listed for GWT 2.8.2 is
Missing bounds check in String.charAt.Sure enough this is causing some exceptions in our code where we try to get chars off the end of a string (or before in one case, i.e. -1).
Fixes #4416
Also fixes case where typing with cursor at column zero of the Console would throw an exception.
I did a quick survey of the uses of
charAtand put in some preventative measures, but certainly possible there are things I missed so we should keep an eye out for this.