8303623: Compiler should disallow non-standard UTF-8 string encodings

Reviewed-by: vromero

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java

@@ -236,6 +236,7 @@ public enum Feature {
  REDUNDANT_STRICTFP(JDK17),
  UNCONDITIONAL_PATTERN_IN_INSTANCEOF(JDK19, Fragments.FeatureUnconditionalPatternsInInstanceof, DiagKind.PLURAL),
  RECORD_PATTERNS(JDK19, Fragments.FeatureDeconstructionPatterns, DiagKind.PLURAL),
+ WARN_ON_ILLEGAL_UTF8(MIN, JDK21),
  ;
 
  enum DiagKind {

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java

@@ -2453,7 +2453,12 @@ protected void append(char ch) {
 
  @Override
  protected void append(byte[] ba) {
- Name name = names.fromUtf(ba);
+ Name name;
+ try {
+ name = names.fromUtf(ba);
+ } catch (InvalidUtfException e) {
+ throw new AssertionError(e);
+ }
  sb.append(name.toString());
  }
 

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java

@@ -551,7 +551,7 @@ class RuntimeEnumMapping implements EnumMapping {
  .fromString(target.syntheticNameChar() +
  "SwitchMap" +
  target.syntheticNameChar() +
- names.fromUtf(ClassWriter.externalize(forEnum.type.tsym.flatName())).toString()
+ ClassWriter.externalize(forEnum.type.tsym.flatName().toString())
  .replace('/', '.')
  .replace('.', target.syntheticNameChar()));
  ClassSymbol outerCacheClass = outerCacheClass();

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java

@@ -106,6 +106,7 @@ public class ClassFile {
 
  public enum Version {
  V45_3(45, 3), // base level for all attributes
+ V48(48, 0), // JDK 1.4
  V49(49, 0), // JDK 1.5: enum, generics, annotations
  V50(50, 0), // JDK 1.6: stackmaps
  V51(51, 0), // JDK 1.7
@@ -165,24 +166,18 @@ public static byte[] internalize(byte[] buf, int offset, int len) {
  * Note: the naming is the inverse of that used by JVMS 4.2 The Internal Form Of Names,
  * which defines "internal name" to be the form using "/" instead of "."
  */
- public static byte[] internalize(Name name) {
- return internalize(name.getByteArray(), name.getByteOffset(), name.getByteLength());
+ public static Name internalize(Name name) {
+ return name.table.names.fromString(name.toString().replace('/', '.'));
  }
 
  /**
- * Return external representation of buf[offset..offset+len-1], converting '.' to '/'.
+ * Return external representation of given name, converting '/' to '.'.
  *
  * Note: the naming is the inverse of that used by JVMS 4.2 The Internal Form Of Names,
  * which defines "internal name" to be the form using "/" instead of "."
  */
- public static byte[] externalize(byte[] buf, int offset, int len) {
- byte[] translated = new byte[len];
- for (int j = 0; j < len; j++) {
- byte b = buf[offset + j];
- if (b == '.') translated[j] = (byte) '/';
- else translated[j] = b;
- }
- return translated;
+ public static Name externalize(Name name) {
+ return name.table.names.fromString(externalize(name.toString()));
  }
 
  /**
@@ -191,7 +186,7 @@ public static byte[] externalize(byte[] buf, int offset, int len) {
  * Note: the naming is the inverse of that used by JVMS 4.2 The Internal Form Of Names,
  * which defines "internal name" to be the form using "/" instead of "."
  */
- public static byte[] externalize(Name name) {
- return externalize(name.getByteArray(), name.getByteOffset(), name.getByteLength());
+ public static String externalize(String name) {
+ return name.replace('.', '/');
  }
 }

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java

@@ -43,6 +43,7 @@
 import javax.tools.JavaFileManager;
 import javax.tools.JavaFileObject;
 
+import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.code.Source.Feature;
 import com.sun.tools.javac.comp.Annotate;
 import com.sun.tools.javac.comp.Annotate.AnnotationTypeCompleter;
@@ -65,6 +66,7 @@
 import com.sun.tools.javac.util.ByteBuffer.UnderflowException;
 import com.sun.tools.javac.util.DefinedBy.Api;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
+import com.sun.tools.javac.util.JCDiagnostic.Fragment;
 
 import static com.sun.tools.javac.code.Flags.*;
 import static com.sun.tools.javac.code.Kinds.Kind.*;
@@ -118,6 +120,10 @@ public class ClassReader {
  */
  boolean lintClassfile;
 
+ /** Switch: warn (instead of error) on illegal UTF-8
+ */
+ boolean warnOnIllegalUtf8;
+
  /** Switch: preserve parameter names from the variable table.
  */
  public boolean saveParameterNames;
@@ -190,6 +196,9 @@ public class ClassReader {
  /** The minor version number of the class file being read. */
  int minorVersion;
 
+ /** UTF-8 validation level */
+ Convert.Validation utf8validation;
+
  /** A table to hold the constant pool indices for method parameter
  * names, as given in LocalVariableTable attributes.
  */
@@ -281,6 +290,7 @@ protected ClassReader(Context context) {
  allowModules = Feature.MODULES.allowedInSource(source);
  allowRecords = Feature.RECORDS.allowedInSource(source);
  allowSealedTypes = Feature.SEALED_CLASSES.allowedInSource(source);
+ warnOnIllegalUtf8 = Feature.WARN_ON_ILLEGAL_UTF8.allowedInSource(source);
 
  saveParameterNames = options.isSet(PARAMETERS);
 
@@ -307,10 +317,18 @@ private void enterMember(ClassSymbol c, Symbol sym) {
  ***********************************************************************/
 
  public ClassFinder.BadClassFile badClassFile(String key, Object... args) {
+ return badClassFile(diagFactory.fragment(key, args));
+ }
+
+ public ClassFinder.BadClassFile badClassFile(Fragment fragment) {
+ return badClassFile(diagFactory.fragment(fragment));
+ }
+
+ public ClassFinder.BadClassFile badClassFile(JCDiagnostic diagnostic) {
  return new ClassFinder.BadClassFile (
  currentOwner.enclClass(),
  currentClassFile,
- diagFactory.fragment(key, args),
+ diagnostic,
  diagFactory,
  dcfh);
  }
@@ -335,7 +353,7 @@ char nextChar() {
  try {
  res = buf.getChar(bp);
  } catch (UnderflowException e) {
- throw badClassFile("bad.class.truncated.at.offset", Integer.toString(e.getLength()));
+ throw badClassFile(Fragments.BadClassTruncatedAtOffset(e.getLength()));
  }
  bp += 2;
  return res;
@@ -347,7 +365,7 @@ int nextByte() {
  try {
  return buf.getByte(bp++) & 0xFF;
  } catch (UnderflowException e) {
- throw badClassFile("bad.class.truncated.at.offset", Integer.toString(e.getLength()));
+ throw badClassFile(Fragments.BadClassTruncatedAtOffset(e.getLength()));
  }
  }
 
@@ -358,7 +376,7 @@ int nextInt() {
  try {
  res = buf.getInt(bp);
  } catch (UnderflowException e) {
- throw badClassFile("bad.class.truncated.at.offset", Integer.toString(e.getLength()));
+ throw badClassFile(Fragments.BadClassTruncatedAtOffset(e.getLength()));
  }
  bp += 4;
  return res;
@@ -455,7 +473,7 @@ Type sigToType() {
  sigp++;
  return sigEnterPhase
  ? Type.noType
- : findTypeVar(names.fromUtf(signature, start, sigp - 1 - start));
+ : findTypeVar(readName(signature, start, sigp - 1 - start));
  case '+': {
  sigp++;
  Type t = sigToType();
@@ -539,8 +557,7 @@ Type sigToType() {
  typevars = typevars.leave();
  return poly;
  default:
- throw badClassFile("bad.signature",
- Convert.utf2string(signature, sigp, 10));
+ throw badClassFile("bad.signature", quoteBadSignature());
  }
  }
 
@@ -550,8 +567,7 @@ Type sigToType() {
  */
  Type classSigToType() {
  if (signature[sigp] != 'L')
- throw badClassFile("bad.class.signature",
- Convert.utf2string(signature, sigp, 10));
+ throw badClassFile("bad.class.signature", quoteBadSignature());
  sigp++;
  Type outer = Type.noType;
  int startSbp = sbp;
@@ -561,7 +577,7 @@ Type classSigToType() {
  switch (c) {
 
  case ';': { // end
- ClassSymbol t = enterClass(names.fromUtf(signatureBuffer,
+ ClassSymbol t = enterClass(readName(signatureBuffer,
  startSbp,
  sbp - startSbp));
 
@@ -575,7 +591,7 @@ Type classSigToType() {
  }
 
  case '<': // generic arguments
- ClassSymbol t = enterClass(names.fromUtf(signatureBuffer,
+ ClassSymbol t = enterClass(readName(signatureBuffer,
  startSbp,
  sbp - startSbp));
  outer = new ClassType(outer, sigToTypes('>'), t) {
@@ -638,7 +654,7 @@ public void setEnclosingType(Type outer) {
  case '.':
  //we have seen an enclosing non-generic class
  if (outer != Type.noType) {
- t = enterClass(names.fromUtf(signatureBuffer,
+ t = enterClass(readName(signatureBuffer,
  startSbp,
  sbp - startSbp));
  outer = new ClassType(outer, List.nil(), t);
@@ -655,6 +671,20 @@ public void setEnclosingType(Type outer) {
  }
  }
 
+ /** Quote a bogus signature for display inside an error message.
+ */
+ String quoteBadSignature() {
+ String sigString;
+ try {
+ sigString = Convert.utf2string(signature, sigp, siglimit - sigp, Convert.Validation.NONE);
+ } catch (InvalidUtfException e) {
+ throw new AssertionError(e);
+ }
+ if (sigString.length() > 32)
+ sigString = sigString.substring(0, 32) + "...";
+ return "\"" + sigString + "\"";
+ }
+
  /** Convert (implicit) signature to list of types
  * until `terminator' is encountered.
  */
@@ -701,7 +731,7 @@ List<Type> sigToTypeParams() {
  Type sigToTypeParam() {
  int start = sigp;
  while (signature[sigp] != ':') sigp++;
- Name name = names.fromUtf(signature, start, sigp - start);
+ Name name = readName(signature, start, sigp - start);
  TypeVar tvar;
  if (sigEnterPhase) {
  tvar = new TypeVar(name, currentOwner, syms.botType);
@@ -752,6 +782,19 @@ Type findTypeVar(Name name) {
  }
  }
 
+ private Name readName(byte[] buf, int off, int len) {
+ try {
+ return names.fromUtf(buf, off, len, utf8validation);
+ } catch (InvalidUtfException e) {
+ if (warnOnIllegalUtf8) {
+ log.warning(Warnings.InvalidUtf8InClassfile(currentClassFile,
+ Fragments.BadUtf8ByteSequenceAt(sigp)));
+ return names.fromUtfLax(buf, off, len);
+ }
+ throw badClassFile(Fragments.BadUtf8ByteSequenceAt(sigp));
+ }
+ }
+
 /************************************************************************
  * Reading Attributes
  ***********************************************************************/
@@ -1106,7 +1149,7 @@ protected void read(Symbol sym, int attrLen) {
  ModuleSymbol msym = (ModuleSymbol) sym.owner;
  ListBuffer<Directive> directives = new ListBuffer<>();
 
- Name moduleName = poolReader.peekModuleName(nextChar(), names::fromUtf);
+ Name moduleName = poolReader.peekModuleName(nextChar(), ClassReader.this::readName);
  if (currentModule.name != moduleName) {
  throw badClassFile("module.name.mismatch", moduleName, currentModule.name);
  }
@@ -1201,8 +1244,18 @@ protected void read(Symbol sym, int attrLen) {
  }
  }
 
- private Name classNameMapper(byte[] arr, int offset, int length) {
- return names.fromUtf(ClassFile.internalize(arr, offset, length));
+ private Name classNameMapper(byte[] arr, int offset, int length) throws InvalidUtfException {
+ byte[] buf = ClassFile.internalize(arr, offset, length);
+ try {
+ return names.fromUtf(buf, 0, buf.length, utf8validation);
+ } catch (InvalidUtfException e) {
+ if (warnOnIllegalUtf8) {
+ log.warning(Warnings.InvalidUtf8InClassfile(currentClassFile,
+ Fragments.BadUtf8ByteSequenceAt(e.getOffset())));
+ return names.fromUtfLax(buf, 0, buf.length);
+ }
+ throw e;
+ }
  }
  },
 
@@ -1502,7 +1555,7 @@ void readParameterAnnotations(Symbol meth) {
  try {
  numParameters = buf.getByte(bp++) & 0xFF;
  } catch (UnderflowException e) {
- throw badClassFile("bad.class.truncated.at.offset", Integer.toString(e.getLength()));
+ throw badClassFile(Fragments.BadClassTruncatedAtOffset(e.getLength()));
  }
  if (parameterAnnotations == null) {
  parameterAnnotations = new ParameterAnnotations[numParameters];
@@ -1796,7 +1849,7 @@ Attribute readAttributeValue() {
  try {
  c = (char)buf.getByte(bp++);
  } catch (UnderflowException e) {
- throw badClassFile("bad.class.truncated.at.offset", Integer.toString(e.getLength()));
+ throw badClassFile(Fragments.BadClassTruncatedAtOffset(e.getLength()));
  }
  switch (c) {
  case 'B':
@@ -2659,6 +2712,7 @@ private void readClassBuffer(ClassSymbol c) throws IOException {
  Integer.toString(maxMajor),
  Integer.toString(maxMinor));
  }
+ utf8validation = majorVersion < V48.major ? Convert.Validation.PREJDK14 : Convert.Validation.STRICT;
 
  if (previewClassFile) {
  if (!preview.isEnabled()) {

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ModuleNameReader.java

@@ -157,11 +157,13 @@ int nextInt() throws BadClassFile {
  return res;
  }
 
- NameMapper<String> utf8Mapper(boolean internalize) {
+ PoolReader.Utf8Mapper<String> utf8Mapper(boolean internalize) {
  return internalize ?
- (buf, offset, len) ->
- Convert.utf2string(ClassFile.internalize(buf, offset, len)) :
- Convert::utf2string;
+ (buf, offset, len) -> {
+ buf = ClassFile.internalize(buf, offset, len);
+ return Convert.utf2string(buf, 0, buf.length, Convert.Validation.STRICT);
+ } :
+ (buf, offset, len) -> Convert.utf2string(buf, offset, len, Convert.Validation.STRICT);
  }
 
 }