----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15958/#review29722 -----------------------------------------------------------
Ship it! LGTM trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java <https://reviews.apache.org/r/15958/#comment57181> I know you're simply fixing this code and didn't write it, but I'm not a huge fan of the approach used here. It would be much cleaner in this case to use a Google common's "Joiner.on(",").join(list);" That one line could replace this whole method. - Stanton Sievers On Dec. 3, 2013, 10:25 a.m., Andreas Kohn wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15958/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2013, 10:25 a.m.) > > > Review request for shindig. > > > Bugs: SHINDIG-1958 > https://issues.apache.org/jira/browse/SHINDIG-1958 > > > Repository: shindig > > > Description > ------- > > Fix for > > SEVERE: Servlet.service() for servlet [OAuth2Servlet] in context with path > [/api] threw exception [java.lang.StringIndexOutOfBoundsException: String > index out of range: 34] with root cause > java.lang.StringIndexOutOfBoundsException: String index out of range: 34 > at > java.lang.AbstractStringBuilder.deleteCharAt(AbstractStringBuilder.java:762) > at java.lang.StringBuilder.deleteCharAt(StringBuilder.java:258) > at > org.apache.shindig.social.core.oauth2.OAuth2TokenHandler.listToString(OAuth2TokenHandler.java:94) > at > org.apache.shindig.social.core.oauth2.OAuth2TokenHandler.handle(OAuth2TokenHandler.java:73) > at > org.apache.shindig.social.core.oauth2.OAuth2Servlet.doGet(OAuth2Servlet.java:71) > at javax.servlet.http.HttpServlet.service(HttpServlet.java:621) > at javax.servlet.http.HttpServlet.service(HttpServlet.java:728) > > Happens when the OAuth2Code has a non-null/non-empty scope set. > > > Diffs > ----- > > > trunk/java/social-api/src/main/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandler.java > 1547331 > > trunk/java/social-api/src/test/java/org/apache/shindig/social/core/oauth2/OAuth2TokenHandlerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/15958/diff/ > > > Testing > ------- > > * Unit test in the patch. > * Use in our application which uses OAuth scopes > > > Thanks, > > Andreas Kohn > >