On 2011/12/08 18:29:14, jlabanca wrote:
LGTM
LGTM
http://gwt-code-reviews.appspot.com/1608803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2011/12/07 19:38:28, mdempsky wrote:
LGTM.
http://gwt-code-reviews.appspot.com/1606804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
Also, jlabanca kindly offered to submit this patch for you.
Thanks!
--xtof
http://gwt-code-reviews.appspot.com/1449814/diff/5/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
File user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
(right):
http
http://gwt-code-reviews.appspot.com/1449814/diff/1/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
File user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
(right):
http://gwt-code-reviews.appspot.com/1447812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
/safehtml/shared/SafeUriHostedModeUtils.java
(working copy)
@@ -104,9 +104,11 @@
}
private static boolean isValidUri(String uri) {
-if (!uri.matches(HREF_UCSCHAR)) {
- return false;
-}
+// TODO(xtof): The regex appears to cause stack overflows in some
cases
http://gwt-code-reviews.appspot.com/1443813/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: tbroyer, jlabanca, jat, pdr, rjrjr,
Description:
Make UriUtils#unsafeCastFromUntrustedString gracefully handle null for
backwards compatibility.
Please review this at http://gwt-code-reviews.appspot.com/1443814/
Affected files:
M
Reviewers: tbroyer, jlabanca, jat, skybrian,
Description:
Add SafeUri type, similar to SafeHtml but for values in a URL attribute
context.
Support SafeUri-typed arguments in SafeHtmlTemplates.
Also added a few overloads in c.g.g.user.client.
Note that this is a breaking change in the sense
http://gwt-code-reviews.appspot.com/1447812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks for the review! Please take another look...
--xtof
http://gwt-code-reviews.appspot.com/1447812/diff/1/tools/api-checker/config/gwt23_24userApi.conf
File tools/api-checker/config/gwt23_24userApi.conf (right):
http://gwt-code-reviews.appspot.com/1447812/diff/1/tools/api-checker/config
/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode432
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:432:
if (isSafeUri(parameterType)) {
On 2011/06/03 17:00:04, jlabanca wrote:
On 2011/06/03 07:39:23, xtof wrote:
On 2011/06/02 13:47:05, jlabanca
http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
Reviewers: tbroyer, jlabanca, jat, skybrian,
Description:
Add SafeUri type, similar to SafeHtml but for values in a URL attribute
context.
Support SafeUri-typed arguments in SafeHtmlTemplates.
Also added a few overloads in c.g.g.user.client.
Note that this is a breaking change in the sense
This is a continuation of the review of
http://gwt-code-reviews.appspot.com/1380806, authored by
http://gwt-code-reviews.appspot.com/user/tbroyer, to capture minor
changes made by the submitter.
For the main code review discussion, see issue 1380806.
On 2011/06/01 00:10:58, tbroyer wrote:
On 2011/05/31 23:38:17, xtof wrote:
My apologies dropping the response on this thread (I'd missed the
last
question and had assumed this was good to submit).
Also, I didn't realize that I actually need to fetch and submit this
patch
(I'm not part
http://gwt-code-reviews.appspot.com/1380806/diff/28033/user/src/com/google/gwt/safehtml/shared/SafeUri.java
File user/src/com/google/gwt/safehtml/shared/SafeUri.java (right):
http://gwt-code-reviews.appspot.com/1380806/diff/28033/user/src/com/google/gwt/safehtml/shared/SafeUri.java#newcode41
/Image.java (right):
http://gwt-code-reviews.appspot.com/1380806/diff/25001/user/src/com/google/gwt/user/client/ui/Image.java#newcode144
user/src/com/google/gwt/user/client/ui/Image.java:144: private String
url = null;
On 2011/04/23 14:36:22, tbroyer wrote:
On 2011/04/18 16:22:32, xtof wrote:
I'm still
LGTM
http://gwt-code-reviews.appspot.com/1422807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks!
just a few more comments, otherwise pretty much LGTM.
http://gwt-code-reviews.appspot.com/1380806/diff/25001/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode302
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:302:
if (HtmlContext.Type.URL_ATTRIBUTE_START == contextType) {
On 2011/04/18 17:03:27, tbroyer wrote:
On 2011/04/18 16:22:32, xtof wrote
LGTM.
http://gwt-code-reviews.appspot.com/1413802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM!
http://gwt-code-reviews.appspot.com/1384801/diff/8001/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
scheme and URI/%-escapes characters not in the allowed charset
(or maybe rejects URLs with certain funny characters in them).
I'll have to do some reading up on what characters are OK in the URI in
a CSS context, will get back to you on that.
Thanks again for all your work on this!
--xtof
http
/SafeHtmlTemplatesImplMethodCreator.java#newcode176
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:176:
// TODO(xtof): Throw an exception if using SafeHtml within an attribute.
I think you can remove this TODO, isn't this handled now in your new
checks at the start
http://gwt-code-reviews.appspot.com/1396803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
of, or in addition
to,
testing isUrlStart, add a test for
HTML5_URI_ATTRIBUTES.contains(streamHtmlParser.getAttribute()).
Thanks for bringing this up. I think addressing this in the parser is
the right place. I know the author of the parser and will bring this up
with him.
Thanks again!
--xtof
http://gwt-code-reviews.appspot.com/1396803/diff/1/user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java
File user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java
(right):
Reviewers: jlabanca, tbroyer,
Description:
Adds support for the URL_ATTRIBUTE_ENTIRE parse context to
HtmlTemplateParser.
Please review this at http://gwt-code-reviews.appspot.com/1396803/
Affected files:
M user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java
M
Reviewers: jlabanca, pdr,
Description:
Adds support for the CSS_ATTRIBUTE_START parse context to
HtmlTemplateParser.
Please review this at http://gwt-code-reviews.appspot.com/1392801/
Affected files:
M user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java
M
http://gwt-code-reviews.appspot.com/1392801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
(ENTIRE_URL_ATTRIBUTE).
Thanks!
--xtof
http://gwt-code-reviews.appspot.com/1380806/diff/5003/user/src/com/google/gwt/user/client/ui/Image.java
File user/src/com/google/gwt/user/client/ui/Image.java (right):
http://gwt-code-reviews.appspot.com/1380806/diff/5003/user/src/com/google/gwt/user/client
On 2011/03/29 19:30:16, jlabanca wrote:
LGTM
I don't actually see the TODO though. You can add it before
submitting.
Oops, forgot to export git before uploading. Done.
Thanks!
--xtof
http://gwt-code-reviews.appspot.com/1392801/
--
http://groups.google.com/group/Google-Web-Toolkit
http://gwt-code-reviews.appspot.com/1392801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
/SafeHtmlTemplatesImplMethodCreator.java
(revision 9912)
+++
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(working copy)
@@ -245,6 +245,7 @@
break;
case CSS_ATTRIBUTE:
+ case CSS_ATTRIBUTE_START:
// TODO(xtof): Improve support for CSS
http://gwt-code-reviews.appspot.com/1380806/diff/1/user/src/com/google/gwt/cell/client/ImageCell.java
File user/src/com/google/gwt/cell/client/ImageCell.java (right):
http://gwt-code-reviews.appspot.com/1380806/diff/1/user/src/com/google/gwt/cell/client/ImageCell.java#newcode58
http://gwt-code-reviews.appspot.com/1384801/diff/3008/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
http://gwt-code-reviews.appspot.com/1384801/diff/6006/user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
File user/src/com/google/gwt/safecss/shared/SafeCssProperties.java
(right):
This is really great! It pretty much completely removes uibinder out of
the security-relevant codebase.
http://gwt-code-reviews.appspot.com/1305801/diff/55001/user/src/com/google/gwt/uibinder/elementparsers/HtmlMessageInterpreter.java
File
http://gwt-code-reviews.appspot.com/1251801/diff/44001/45010
File user/src/com/google/gwt/user/server/Util.java (right):
http://gwt-code-reviews.appspot.com/1251801/diff/44001/45010#newcode76
user/src/com/google/gwt/user/server/Util.java:76: * @throws
IllegalStateException if duplicate cookies
Hey Meder,
looks good! I have a couple more comments, also in the tests which I
hadn't looked at in detail yet...
cheers --xtof
http://gwt-code-reviews.appspot.com/1251801/diff/30001/31010
File user/src/com/google/gwt/user/server/Util.java (right):
http://gwt-code-reviews.appspot.com
http://gwt-code-reviews.appspot.com/1251801/diff/30001/31010
File user/src/com/google/gwt/user/server/Util.java (right):
http://gwt-code-reviews.appspot.com/1251801/diff/30001/31010#newcode76
user/src/com/google/gwt/user/server/Util.java:76: if (cookieName == null
|| request == null) {
On
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005
File
user/src/com/google/gwt/user/server/rpc/AbstractXsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005#newcode80
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009
File
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009#newcode192
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java:192:
:120:
private boolean isCookieValueValid(String cookieValue) {
On 2011/01/11 01:48:15, meder wrote:
On 2011/01/11 00:32:23, xtof wrote:
I'm not too fond of how this method performs two different kinds of
functions,
depending on how the class is configured:
If the class is configured to use
Reviewers: rice,
Description:
Expand self-closing div/ tag in SafeHtmlTemplate into separate open
and close
tag.
Please review this at http://gwt-code-reviews.appspot.com/1209801/show
Affected files:
M user/src/com/google/gwt/user/cellview/client/CellTable.java
Index:
LGTM!
http://gwt-code-reviews.appspot.com/1107801/diff/3001/4005
File user/src/com/google/gwt/user/client/rpc/ServiceDefTarget.java
(right):
http://gwt-code-reviews.appspot.com/1107801/diff/3001/4005#newcode43
user/src/com/google/gwt/user/client/rpc/ServiceDefTarget.java:43:
RpcToken
Reviewers: rice, pdr,
Description:
Document limitations of current SafeHtmlTemplates code generator.
Please review this at http://gwt-code-reviews.appspot.com/1006801/show
Affected files:
M user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java
Index:
http://gwt-code-reviews.appspot.com/1006801/diff/1/2
File user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java
(right):
http://gwt-code-reviews.appspot.com/1006801/diff/1/2#newcode53
user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java:53: *
{...@code @Template} annotation
http://gwt-code-reviews.appspot.com/1006801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/850801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM!
http://gwt-code-reviews.appspot.com/847801/diff/8002/13005
File user/src/com/google/gwt/user/client/ui/Anchor.java (right):
http://gwt-code-reviews.appspot.com/847801/diff/8002/13005#newcode117
user/src/com/google/gwt/user/client/ui/Anchor.java:117:
this(html.asString(), true);
This is
http://gwt-code-reviews.appspot.com/850801/diff/1/11
File user/test/com/google/gwt/safehtml/SafeHtmlGwtSuite.java (right):
http://gwt-code-reviews.appspot.com/850801/diff/1/11#newcode22
user/test/com/google/gwt/safehtml/SafeHtmlGwtSuite.java:22: import
LGTM
http://gwt-code-reviews.appspot.com/829801/diff/8020/24004
File user/src/com/google/gwt/user/client/ui/CheckBox.java (right):
http://gwt-code-reviews.appspot.com/829801/diff/8020/24004#newcode265
user/src/com/google/gwt/user/client/ui/CheckBox.java:265: public void
setSafeHtml(SafeHtml
Reviewers: pdr, rice,
Description:
Cosmetic changes in safehtml package.
Run SafeHtmlBuilderTest both as a GWT and a JRE test.
Please review this at http://gwt-code-reviews.appspot.com/821801/show
Affected files:
M user/src/com/google/gwt/safehtml/shared/SafeHtmlBuilder.java
M
http://gwt-code-reviews.appspot.com/821801/diff/1/2
File user/src/com/google/gwt/safehtml/shared/SafeHtmlBuilder.java
(right):
http://gwt-code-reviews.appspot.com/821801/diff/1/2#newcode81
user/src/com/google/gwt/safehtml/shared/SafeHtmlBuilder.java:81: *
@param c the number whose string
http://gwt-code-reviews.appspot.com/821801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/796802/diff/1/10
File user/src/com/google/gwt/i18n/rebind/MessagesMethodCreator.java
(right):
http://gwt-code-reviews.appspot.com/796802/diff/1/10#newcode425
user/src/com/google/gwt/i18n/rebind/MessagesMethodCreator.java:425:
private boolean returnsSafeHtml;
LGTM
http://gwt-code-reviews.appspot.com/793801/diff/1/2
File user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java
(right):
http://gwt-code-reviews.appspot.com/793801/diff/1/2#newcode42
user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java:42: *
SafeHtmlLabel
LGTM
http://gwt-code-reviews.appspot.com/799801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/793801/diff/1/2
File user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java
(right):
http://gwt-code-reviews.appspot.com/793801/diff/1/2#newcode55
user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java:55:
public interface SafeHtmlTemplates {
http://gwt-code-reviews.appspot.com/771801/diff/1/10
File user/src/com/google/gwt/safehtml/shared/EscapeUtils.java (right):
http://gwt-code-reviews.appspot.com/771801/diff/1/10#newcode29
user/src/com/google/gwt/safehtml/shared/EscapeUtils.java:29: * Returns a
SafeHtml constructed from a safe
On 2010/08/17 23:05:06, tbroyer wrote:
On 2010/08/17 20:19:04, xtof wrote:
On 2010/08/17 18:39:28, jat wrote:
On 2010/08/16 23:39:47, tbroyer wrote:
The HtmlSanitizer is a good idea, but the implementation is very
weak [2].
Note that the API is what is important
63 matches
Mail list logo
