Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Breaking java.lang.String

wouter.coekaerts.be

Post reply on HN
Print view View original

181–190 of 206 posts

Re: Breaking java.lang.String

#181

Post by Dylan16807 »

dundarious wrote: post #168
Dylan16807 wrote: post #164

Earlier quoted context omitted.

It only loops if you modify the string in certain ways partway through the loop. Is that a significant problem? As soon as you stop your indefinite loop of race-condition writes, this loop is guaranteed to finish.

Well it's trading "bad code can populate the program's string intern table with invalid string objects" for "bad code can instantly deadlock the program", which is not much of an upgrade. And wouldn't you need to do this in every function that uses more than 2 or more related mutable objects 1 time each, or uses 1 mutable object more than 1 time? Do you know of any systems that work like this? This is basically a ver…

It has to be almost deliberately bad to cause a deadlock.

This solution stops you from having random occasional breakage.

Re: Breaking java.lang.String

#182

Post by Genbox »

coekie wrote: post #75
gizmo686 wrote: post #14

Is this actually a bug? The default assumption in Java is that types are not thread-safe unless otherwise specified. Attempting to use types in a way that exceeds their documented thread safety has always been allowed to leave your program in an inconsistent state.

Author of the blog here. Yes, this is a bug. While the javadoc doesn't state it explicitly, immutable classes in the core library are expected to be thread safe. Java tries to be and mostly succeeds at being a safe language, where (by default) the guarantees of its internals cannot be broken no matter what user code does. The JVM preserves its own integrity. There are some other deliberate holes in that safety, such…

Immutability and thread-safety are two different things. There are no thread-safety guarantees made by the JVM or the runtime for the String type, and I suspect your bug report will get the same answer.

You are right that there is an expecation or assumption of such (from the users), which makes it even more interesting when it breaks.

Locking a thread for every allocation of a string is very likely cost-prohibitive and not a great trade-off for the common case. It would make more sense to add something like StringFactory.CreateString(char[]) and instruct developers in using it if thread safety is nessecary.

Re: Breaking java.lang.String

#183

Post by ris58h »

layer8 wrote: post #169
ris58h wrote: post #151

Earlier quoted context omitted.

So they should add that mutating arguments during object construction may lead to not-so-successfully constructed (invalid) object. __Mission failed successfully__

No. The job of a constructor is to establish the class’s invariants for the new instance, or else fail with an exception. This bug in the String class fails to do so.

> The job of a constructor is to establish the class’s invariants for the new instance, or else fail with an exception.

Could you refer such statement in Java specification?

Re: Breaking java.lang.String

#184

Post by barrkel »

dundarious wrote: post #144
barrkel wrote: post #116

Earlier quoted context omitted.

This bug can change the meaning of code in third party libraries by altering the set of interned strings. If any input to a program can be converted into this race, then it becomes a security vulnerability. It would already be a security vulnerability in the old Java sandbox model.

If you have a data race, your program can have all sorts of inconsistent state in all sorts of objects -- even standard ones. In Java, I don't think you have anything like "undefined behavior", sometimes jokingly but meaningfully called catch-fire or launch-the-nukes semantics, but that doesn't prevent a data race from breaking program invariants in ways that don't immediately or ever generate exceptions. There was a…

That's a lot of talking around the issue and I think you're missing it.

We're talking about constants being corrupted in code which has has no control or data flow relationship with the code doing the corrupting. Code which is totally bug free can exhibit impossible behavior.

That might not sound disturbing to you if you're taking about strlen(), because languages with unrestricted pointers are full of undefined behavior. But Java isn't like that. Java doesn't come with nasal demons.

I didn't say intern() has a bug.

The bug is Java having undefined behavior of the kind which breaks a language invariant as strong as string equality.

You're arguing that there isn't a bug because non-determinism comes from concurrency. Well, you can't have concurrency without non-determinism. That doesn't mean the language needs to stop working.

Re: Breaking java.lang.String

#185

Post by vacuity »

masklinn wrote: post #171
vacuity wrote: post #136

Earlier quoted context omitted.

While that is good advice, it can't apply to mutable input. Unless a defensive copy is made (at least, under the current system), concurrent modification can still occur. The new type's underlying data is still being accessed.

There are necessarily copies being made since it's converting a `char[]` to a `byte[]`, the problem is that they're not done correctly. Currently the code tries to encode the chars, and if it fails it completely bails out and restarts with a code unit copy. The bailing and restarting is what offers the opportunity for TOCTOU. But if instead of bailing it converted the data collected so far to code units, then appende…

Ah, good points. Thank you for the insight!

Re: Breaking java.lang.String

#186

Post by barrkel »

pshirshov wrote: post #118
barrkel wrote: post #113

Earlier quoted context omitted.

A defensive copy is the cheap way to fix this. The bug is a potential security vulnerability since it can affect the value of interned strings, which can break other libraries.

That still has to be done ad-hoc and might kill performance for many usecases. This is not a bug, but a specified behavior.

Changing the evaluation of the expression x.equals(y) to false when x and y are strings with the same value is not a specified behavior.

Re: Breaking java.lang.String

#187

Post by pshirshov »

barrkel wrote: post #186
pshirshov wrote: post #118

Earlier quoted context omitted.

That still has to be done ad-hoc and might kill performance for many usecases. This is not a bug, but a specified behavior.

Changing the evaluation of the expression x.equals(y) to false when x and y are strings with the same value is not a specified behavior.

It is. The "buggy" code isn't marked as thread safe.

Re: Breaking java.lang.String

#188

Post by thfuran »

tialaramex wrote: post #155
thfuran wrote: post #143

Earlier quoted context omitted.

Yes, ArrayLists are array-backed, but Arrays.asList instantiates a List and does not instantiate an array.

As I explained, List is an interface. You can't instantiate it, values are instances of types not interfaces . All that return type is telling you is, surprise, asList promises what you're getting implements the List interface. See if you can guess how you can implement the List interface using an Array. Still struggling, here's the one line of a typical implementation: return new Arrays.ArrayList(a);

You should at least try to be correct if you're going to be so insufferable about it.

Re: Breaking java.lang.String

#189

Post by jdmichal »

ris58h wrote: post #183
layer8 wrote: post #169

Earlier quoted context omitted.

No. The job of a constructor is to establish the class’s invariants for the new instance, or else fail with an exception. This bug in the String class fails to do so.

> The job of a constructor is to establish the class’s invariants for the new instance, or else fail with an exception. Could you refer such statement in Java specification?

Invariants and all methods -- including constructors -- enforcing them is OOP 101. I would be surprised if they felt the need to make such a callout in the language specification.

Re: Breaking java.lang.String

#190

Post by erik_seaberg »

kevincox wrote: post #132
Zach_the_Lizard wrote: post #130

Earlier quoted context omitted.

You can also use const_cast to remove the const-ness. You could then mutate the value. I believe that's undefined behavior territory (the mutation, at least in most cases), but I'm sure someone is doing it in the wild.

I haven't done much C++ in a few years but IIRC you can remove const as long as the "original" value isn't const. So `const_cast((*const Foo)foo)` is fine if foo is not const.

Isn’t the linker entitled to put a constant object in a read-only page of the binary if it doesn’t require a ctor at runtime?
Post reply on HN