Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 22 Mar 2014, at 03:22, Mandy Chung mandy.ch...@oracle.com wrote: Good catch Sherman. Vinnie - what's your recommendation for this LDAP change having both encode/decode uses the platform default charset (rather than retaining the old interop issue)? It's an incompatible change and I'll file a CCC to track this. Yes. I think that’s the right approach as the compatibility risk is low. Mandy On 3/21/2014 2:23 AM, Vincent Ryan wrote: You’re right but we’ve never received a report of any charset interop. issues. Probably such a scenario has never been encountered by customers. On 21 Mar 2014, at 05:54, Xueming Shen xueming.s...@oracle.com wrote: Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
You’re right but we’ve never received a report of any charset interop. issues. Probably such a scenario has never been encountered by customers. On 21 Mar 2014, at 05:54, Xueming Shen xueming.s...@oracle.com wrote: Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
Good catch Sherman. Vinnie - what's your recommendation for this LDAP change having both encode/decode uses the platform default charset (rather than retaining the old interop issue)? It's an incompatible change and I'll file a CCC to track this. Mandy On 3/21/2014 2:23 AM, Vincent Ryan wrote: You’re right but we’ve never received a report of any charset interop. issues. Probably such a scenario has never been encountered by customers. On 21 Mar 2014, at 05:54, Xueming Shen xueming.s...@oracle.com wrote: Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
Fix looks fine. You just need to update the import statements in Obj.java On 19 Mar 2014, at 18:37, Mandy Chung mandy.ch...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
OK. Thanks. On 19 Mar 2014, at 19:06, Mandy Chung mandy.ch...@oracle.com wrote: On 3/19/2014 12:03 PM, Vincent Ryan wrote: Fix looks fine. You just need to update the import statements in Obj.java Fixed. Not sure how it's missed in the webrev :) thanks Mandy On 19 Mar 2014, at 18:37, Mandy Chung mandy.ch...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 3/19/2014 12:03 PM, Vincent Ryan wrote: Fix looks fine. You just need to update the import statements in Obj.java Fixed. Not sure how it's missed in the webrev :) thanks Mandy On 19 Mar 2014, at 18:37, Mandy Chung mandy.ch...@oracle.com wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 19/03/2014 18:37, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Good to see more conversion to the new API, looks good to me. -Alan
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. -Sherman
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 3/19/2014 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Thanks Sherman. I believe both cases don't have the 76 characters constraint although it uses MIME type encoder/decoder previously unless Vinnie and Alan say otherwise. Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks. It calls encode/decode in a loop and so I think keeping a local variable to avoid getting it multiple time seems reasonable. Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 19/03/2014 20:33, Mandy Chung wrote: : I believe both cases don't have the 76 characters constraint although it uses MIME type encoder/decoder previously unless Vinnie and Alan say otherwise. Sherman makes a good point and we should double check the LDAP spec to see which base64 scheme this is (I suspect it is MIME). Vinnie will likely know if it's not often from the RFC. -Alan.
