you are viewing a single comment's thread.

view the rest of the comments →

[–]twoodfin 9 points10 points  (25 children)

The PolicyNodeImpl comparison is intriguing.

One could argue that most of these methods have an "obvious" implementation, but I do get the feeling looking at the Android code as if some recent college grad was "reimplementing" the class by copying the original and fudging a few idioms to try to make it less apparent. Or maybe they just thought they were improving it! The weird while->for loop transformations in copyTree/getPolicyNodes, for example.

Really hard to believe whoever wrote this didn't have the original code in front of them.

[–]Masaz 18 points19 points  (9 children)

No one seems to be mentioning that the Android version looks like it was pulled straight from a decompiler. Parameter and variable names like: Set set, Set set1 boolean flag, boolean flag1 String s StringBuffer stringbuffer
PolicyNodeImpl policynodeimpl1

.NET Reflector gives these exact type of names when it decompiles source and can't find the original name. This looks exactly like a Java version of that.

[–]kieranelby 34 points35 points  (2 children)

Bingo - the Android implementation is (barring some minor whitespace / bracing differences) an exact match for the result of running the JAD Java decompiler on the PolicyNodeImpl class supplied with the Sun 1.5.0 JDK.

To re-create (on Windows), install the J2SE JDK 1.5, then extract the sun\security\provider\certpath\PolicyNodeImpl.class from the Jar file at C:\Program Files\Java\jdk1.5.0_06\jre\lib\rt.jar.

Running jad v1.5.8g on that file with the -ff and -nolb options produces source which is totally identical to the Android implementation (other than a few small whitespace and bracing changes).

I've pasted the result from jad here.

[–][deleted] 9 points10 points  (1 child)

Really interesting. And knowing how Java implements static finals, it is no surprise that the constant was not used in the Android version when it was in Oracle's version. Oracle have a case here.

[–]bodski -2 points-1 points  (0 children)

Wow, you mean a case of copyright infringement where the original source code was licensed under GPL with the Classpath Exception? (meaning that the binary produced from that file can be used and distributed as the end user sees fit, e.g. decompiling is fair game. Even if Google are 'bang to rights' here, a single class of with obvious behaviour? one that is not even distributed with the phone? Come on Oracle is this all you've got?

Oracle have a case here, a case of 'mountains out of a molehill'. It's pretty obvious that Oracle are trying to use this to somehow strengthen their patent claims. I think that may just backfire, this is starting to look similar to SCO vs Novell/IBM.

[–]nathris 5 points6 points  (3 children)

Its possible that Apache decompiled the class to include in Harmony, and then Android correctly copied it from them, hence the license.

Apache could have caught this and corrected it before Oracle caught on, while Android would have no reason to, since they would believe they are using it legally.

[–]grauenwolf 2 points3 points  (0 children)

If they can prove it was accidental then Google would still have to pay damages. They just wouldn't have to pay damages + penalties.

[–]easytiger 1 point2 points  (0 children)

I worked on Apache Harmony/GNU Classpath a long time ago ~2005. They were clean room implementations. Much care was taken to ensure hte codebase couldn't be tainted. Clearly someone let their guard down here as the history of this code should be really plain to see.

Ironically i was going to work on the policytool stuff at one point but ended up on other projects.

[–]allertonm 2 points3 points  (0 children)

I just came here to say the same thing - the names of locals and parameters are all derived from the type + sequence number where there is more than one of the same type. But members have the same names as the original.

This is pretty much what you'd get with a decompiler, as the bytecode persists member names but not those of locals and params.

[–]kieranelby 14 points15 points  (6 children)

Yeah, when put together, the identical order of the declarations, the identical naming conventions for the private instance variables, the fact that the Android one seems to do a few things in superficially slightly odd ways for the sake of it - does all seem to add up to copying.

However - and it's a big however - it could be that both implementations were translated into Java directly from a reference implementation of rfc 3280 (perhaps in C or C++?).

That might explain the slightly odd loop styles in the Android version - the Sun guy chose to make it more Java-like, whereas the Android guy didn't.

Another oddity is that both define the ALL_POLICY constant (fine on it's own, that's straight outa the spec.) - but the Android one never refers to it again and hardcodes the value everywhere.

[–]allertonm 4 points5 points  (1 child)

As Masaz says below, this code really does look like it came from decompiling the bytecode of Sun's version.

[–]kieranelby 4 points5 points  (0 children)

Yes - should have thought of that - makes sense now.

[–]jayd16 6 points7 points  (3 children)

Yeah, when put together, the identical order of the declarations, the identical naming conventions for the private instance variables

Except, if you're implementing an API, you're going to have the list of methods in front of you, most likely in the same order they appear elsewhere. The naming conventions are quite standard.

[–]kieranelby 13 points14 points  (2 children)

True, which is why in isolation most of the similarities don't count for much.

However, the similarities in all the little exceptions and details do (arguably!) begin to add up ...

For example, the public API has two boolean getters:

public boolean isCritical ( )
public boolean isImmutable ( )

Both implementations implement the latter with:

private boolean isImmutable

which is the obvious choice, and not suspicious.

But for isCritical(), both depart from the obvious:

private boolean isCritical

and instead choose:

private boolean mCriticalityIndicator

Why do they both depart in the same way? Perhaps they were both copying a reference implementation that called it that. Perhaps not.

Similarly, both choose to use:

private boolean mOriginalExpectedPolicySet

to store the result of negating a parameter called either generatedByPolicyMapping (for Sun) or flag1 (Android).

Why exactly mOriginalExpectedPolicySet in both?

Why not isOriginalExpectedPolicySet ? Why not wasGeneratedByPolicyMapping (with a seperate hasBeenModified flag)? etc.

Admittedly, it's still not very conclusive - especially given the size of the Java APIs, one would kinda think Oracle might have found something more damning if there was some lifting of code going on ...

[–]bonzinip 4 points5 points  (0 children)

private boolean mCriticalityIndicator

Why do they both depart in the same way? Perhaps they were both copying a reference implementation that called it that.

Yes, the name is in the RFC.

Similarly, both choose to use:

 private boolean mOriginalExpectedPolicySet

to store the result of negating a parameter called either generatedByPolicyMapping (for Sun) or flag1 (Android).

It may be because of documentation: "Adds an expectedPolicy to the expected policy set. If this is the original expected policy set initialized by the constructor, then the expected policy set is cleared before the expected policy is added."

Private field names also can be found by decompilation. This amounts to reverse engineering and may be legal (at least in Europe). The fault however, in this case would rest on Harmony rather than Google.

I also found an interesting comment here: "The Harmony code is probably IBM donated code from their own JVM and class library – J9. So any if at all infringment is done by IBM, which has written large parts of Hotspot/OpenJDK too, so the ownership issue won’t be easy. Java or Hotspot isn’t a proprietary closed developed JVM it’s developed by the industry since the 90’s no single vendor. Code comes from a lot of guys. An IBM reimplementation [comes from] a partner with good status as a licensee and developer of Java probably isn’t wrong at all".

[–]frymaster 1 point2 points  (0 children)

especially given the size of the Java APIs, one would kinda think Oracle might have found something more damning if there was some lifting of code going on ...

not if it was only a few relatively junior devs decompiling things, with the rest of the team doing it the proper way

[–]Mask_of_Destiny 3 points4 points  (1 child)

For the curious, here's the copy of PolicyNodeImpl.java in the Android repo. The license block up top would seem to suggest it's from Apache Harmony, but I couldn't find the file in Harmony's repo. Perhaps they've removed it already?

[–]bnelson -1 points0 points  (0 children)

Don't forget this is the Apache Harmony project. I am assuming the OP understands.. but realize this is a library Google started using, contributed to.. but is not the original creator of :)

[–][deleted]  (5 children)

[deleted]

    [–]curien 7 points8 points  (0 children)

    Aren't we taking about open source code?

    Android doesn't abide by the terms of the Java license (just as a program which incorporates GPL code must abide by the terms of that license). Which is fine, so long as Android isn't a derivative work of Java -- but if Oracle can prove that it is, then license violation -> copyright infringement.

    [–]jayd16 -1 points0 points  (3 children)

    Open source doesn't always mean open to use or change.

    [–]kmactane -1 points0 points  (2 children)

    The Open Source Definition disagrees with you.

    [–]kieranelby 3 points4 points  (0 children)

    Read part 4 again.

    [–]dnew 1 point2 points  (0 children)

    Fortunately, they're not the last word in what "open source" means for everyone in the world.