nats-io / nats-architecture-and-design

Architecture and Design Docs
Apache License 2.0
170 stars 20 forks source link

Make sure that subject is validated in non-hot paths #236

Open Jarema opened 8 months ago

Jarema commented 8 months ago

Overview

Some JetStream API uses values passed by the user as part of the subject. Those values should be validated to:

Examples of affected fields:

More to be added.

we are not yet decided on validating publish hot paths subjects, as we need to perform some benchmarks

The behavior is documented in ADR-X.

Clients and Tools

Other Tasks

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

aricart commented 8 months ago

Can we get clarification - currently, the javascript clients validate:

function minValidation(context: string, name = "") {
  // minimum validation on streams/consumers matches nats cli
  if (name === "") {
    throw Error(`${context} name required`);
  }
  const bad = [".", "*", ">", "/", "\\"];
  bad.forEach((v) => {
    if (name.indexOf(v) !== -1) {
      throw Error(
        `invalid ${context} name - ${context} name cannot contain '${v}'`,
      );
    }
  });
  return "";
}

Recently the validation that rejected spaces in the middle of the name had to be reverted, as some users already have constructs having such names.

scottf commented 8 months ago

this is java/.net validation. These clients limit subjects on management and subscribe to printable characters. I can remove the printable restriction if required, it would be backward compatible.

String[] segments = subject.split("\\.");
for (int x = 0; x < segments.length; x++) {
    String segment = segments[x];
    if (segment.equals(">")) {
        if (cantEndWithGt || x != segments.length - 1) { // if it can end with gt, gt must be last segment
            throw new IllegalArgumentException(label + " cannot contain '>'");
        }
    }
    else if (!segment.equals("*") && notPrintable(segment)) {
        throw new IllegalArgumentException(label + " must be printable characters only.");
    }
}
ripienaar commented 8 months ago

CLI/terraform/GitHub does this for names

func IsValidName(n string) bool {
    if n == "" {
        return false
    }

    return !strings.ContainsAny(n, ">*. /\\")
}