On 2011/05/03 16:11:37, xtof wrote:
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
user/src/com/google/gwt/safehtml/shared/SafeUri.java:41: * context in
which the
URL is used matters too (link {@code href} vs. image
On 2011/05/02 18:57:13, skybrian wrote:
> This sounds somewhat dangerous, actually. It seems like if context
matters,
> either each context should have its own type, or creators of SafeUrl
instances
> should stick to the lowest common denominator.
>
> The reason is that the code creating a SafeUrl instance might be far
removed
> from the code that uses it. If the creator believes that a URL will
be used in
> iframe context, but actually it isn't, then reviewers cannot find
the problem
> without having an end-to-end understanding of the program's data
flow, and any
> refactoring of intermediate code has the potential to introduce a
bug without
a
> type error and without the reviewers seeing anything wrong.
>
> It seems like the whole point of having safe types with clear
contracts is
make
> sure that local reviews are sufficient to guarantee safety?
>
> I hate the complexity this is likely to introduce, but on the other
hand, a
> SafeUrl type that isn't actually safe doesn't sound so great either.
I think I agree with Brian on this one. Can we specify the contract
such that a
SafeUri is safe to use in any of these contexts (at the expense of
being overly
restrictive, for e.g. img src URIs)? If not, I think we need to
introduces
separate types. Otherwise we'll loose the local revieability benefit
of the
SafeXyz types.
If that's the only point left, can you make the edit yourself before
submitting? (as, while I agree, I don't know what to say exactly here)
Otherwise, if there are other comments requiring a new patchset, then
I'll try to address this one at the same time.
TIA
http://gwt-code-reviews.appspot.com/1380806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
