Update to my explanation. String.intern() does refer to this portion of the
Java Language Specification:

https://docs.oracle.com/javase/specs/jls/se7/html/jls-3.html#jls-3.10.5

"Moreover, a string literal always refers to the *same* instance of class
String. This is because string literals - or, more generally, strings that
are the values of constant expressions (ยง15.28
<https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.28>)
- are "interned" so as to share unique instances, using the method
String.intern."


So, there is a language specification support for the understanding that
the constant string "" from one class, will be both String.equals() and ==
another constant string "" in the same Java runtime.

However, there is nothing in the Jetty documentation that specifies that
the return result of .getParameter() will be a constant string when it is
empty. So, any code that assumes this is susceptible to breakage with any
change to the Jetty implementation. Even a very small indirect change could
influence whether or not it *happens* to return a constant string or not. I
stand firmly by my stance that such code should never have passed code
review. That something happens to work, isn't sufficient quality code for
deployment, and it certainly isn't a good reason to restore the old
behaviour.

Despite it being a really bad assumption... you piqued my curiosity as to
exactly how it might have happened that the behaviour changed.

I think this was the commit that "broke" you:

https://github.com/eclipse/jetty.project/commit/866960d5d72445e7e214027108716fa7d64b8951

It looks like the code previously had a few hard coded locations where it
would have a fast path for zero string ( l==0 ? "" : ... ), a fast path for
strings that had no decoding requirements, and a slow path for decoding.
Greg replaced several of these with generic code that would would do the
right thing every time in one common place. The replacement code probably
does not special case "if empty string, return constant empty string."

Probably, the behaviour could be restored in one place. But, probably this
would be a very bad precedent to set, as it basically agrees that users can
expect constant literals to be returned everywhere else they might
currently be returned, and this could lead to enabling other bad code to
continue functioning instead of being corrected. I'm tempted to go the
opposite way here - purposefully avoiding the use of String.intern() or
constants, specifically to avoid the case of people relying on undocumented
behaviour. Prevent people from making a mistake in the first place.


On Fri, Jan 4, 2019 at 8:00 AM Mark Mielke <[email protected]> wrote:

> On Fri, Jan 4, 2019 at 2:27 AM kapil gupta <[email protected]> wrote:
>
>> This is for request.getParameter
>>
>> On Fri, Jan 4, 2019 at 12:30 PM kapil gupta <[email protected]>
>> wrote:
>>
>>> We were using Jetty 8 and now trying to upgrade on Jetty 9. The below
>>> piece of code has different results in both versions of Jetty.
>>>
>>> if (a != "")
>>>
>>> Where a is string variable.
>>>
>>> I understand the code is wrong, we should always use a.equals for
>>> comparison. But we have it at so many places, but not sure why Jetty 8 and
>>> Jetty 9 have different results for it.
>>>
>>
>
> If you think the question above is valid, then the code is so much more
> wrong than you are recognizing. :-)
>
> The "" generates a constant string that is at least in principle allocated
> in the *calling* class. This means it is part of your class. Within your
> class, it is *possible* that the compiler will do de-duplication of the
> constant strings, and emit just one constant string representing "" for
> your whole class, which means that it is *possible* that "" will have the
> same identity anywhere in your class. I emphasize *possible*, as there is
> no language level guarantee for this. This is an invalid assumption that
> should never make it past code review.
>
> Any "" that gets generated from other classes, would either start from a
> constant string defined in another class (with a different identity!), or
> start from a char[0] which will always have a new identity. There is almost
> zero way your code could work in Jetty 8.
>
> I say "almost zero", because there is a way... some Java runtimes may
> de-duplicate constant strings across classes, or during runtime garbage
> collection. This may also be triggered manually by using String.intern()  (
> https://docs.oracle.com/javase/7/docs/api/java/lang/String.html#intern()
> ). In such cases, it's possible that your code might work, sometimes.
>
> "Might work sometimes" dependent upon a runtime feature that was probably
> not understood at the time it was accidentally selected, is a very bad way
> to write code.
>
> You need to fix your code.
>
> --
> Mark Mielke <[email protected]>
>
>

-- 
Mark Mielke <[email protected]>
_______________________________________________
jetty-users mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe from 
this list, visit
https://www.eclipse.org/mailman/listinfo/jetty-users

Reply via email to