Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
3 of 11 tasks
Jarema opened this issue Aug 14, 2023 · 3 comments
Open
3 of 11 tasks

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

Jarema opened this issue Aug 14, 2023 · 3 comments
Assignees
Labels
client Client related work enhancement New feature or request

Comments

@Jarema
Copy link
Member

Jarema commented Aug 14, 2023

Overview

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

  • not contain spaces
  • not contain \r\n or \t
  • not end or start with subject token delimiter .

Examples of affected fields:

  • name and durable when managing consumer
  • name of Stream when managing Stream
  • kv buckets and keys
  • object store buckets

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

  • docs.nats.io updated @bruth
  • Update ADR to Implemented
  • Update client features spreadsheet

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.

@Jarema Jarema added enhancement New feature or request client Client related work labels Aug 14, 2023
@nats-io nats-io deleted a comment from ripienaar Aug 15, 2023
@aricart
Copy link
Member

aricart commented Aug 16, 2023

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
Copy link
Collaborator

scottf commented Aug 16, 2023

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
Copy link
Contributor

ripienaar commented Aug 16, 2023

CLI/terraform/GitHub does this for names

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Client related work enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants