Skip to content

Comments

Remove max label length to avoid corrupting the build.#616

Open
mdealer wants to merge 2 commits intojenkinsci:masterfrom
mdealer:remove-max-label-length
Open

Remove max label length to avoid corrupting the build.#616
mdealer wants to merge 2 commits intojenkinsci:masterfrom
mdealer:remove-max-label-length

Conversation

@mdealer
Copy link

@mdealer mdealer commented Feb 23, 2026

Remove max label length limitation from back-end to avoid corrupting a build with Unicode text in label. Existing implementation may incorrectly cut a multi byte Unicode character and thus make it unreadable during deserialization.

Alternative would be to improve the cutting with graceful handling of Unicode, but that should be up to the UI, not back end. On my end the 100 char limit makes some of my pipelines unreadable as some labels just go slightly over 100 char limit.

Testing done

image

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

Edgars Batna added 2 commits February 19, 2026 14:33
…a unicode char, which corrupts the whole build.

If necessary, this should be done at the view level instead.
@mdealer mdealer marked this pull request as ready for review February 23, 2026 11:44
@mdealer mdealer requested a review from a team as a code owner February 23, 2026 11:44
Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maximum label check is there so that the UI is reasonable and removing this would imply it is ok to support arbitrary long lines in the UI which is incorrect.

So I see 2 choices:

  1. make the max length configurable by a system property (I would discourage this, as it is unlikely to be tested in the UI anywhere) and follow up in the UI so it is truncated correctly / the UI performs correctly.
  2. make the trimming of the String unicode surrogate pair aware

As this label is there for the UI not any backend, I would see that trimming in the backend would be appropriate here.

2 would be something like the following (untested)

private String trim(String source, int maxLen) {
  if(s.length() > maxLen) {
    if (Character.isLowSurrogate(s.charAt(maxLen))) {
      return s.substring(0, maxLen-1);
    }
    return s.substring(0,maxLen);
  }
  return s;
}

@jtnord jtnord requested a review from a team February 23, 2026 12:08
@mdealer
Copy link
Author

mdealer commented Feb 23, 2026

The maximum label check is there so that the UI is reasonable and removing this would imply it is ok to support arbitrary long lines in the UI which is incorrect.

100 characters is quite arbitrary. The UI works just fine, it simply wraps to the next line.

Maybe @jglick can add something? I remember already discussing this here.

Also, to me the build.xml is not UI, it's the back end.

I ran this for a while and I know it works, but I don't feel like cutting:

    public static String unicodeSubstring(String string, int beginIndex, int endIndex) {
        int length = endIndex - beginIndex;
        int[] codePoints = string.codePoints()
            .skip(beginIndex)
            .limit(length)
            .toArray();
        return new String(codePoints, 0, codePoints.length);
    }
    @Override public StepExecution start(StepContext context) throws Exception {
        if (this.label != null) {
            context.get(FlowNode.class).addAction(new LabelAction(label.length() > MAX_LABEL_LENGTH ? unicodeSubstring(label, 0, MAX_LABEL_LENGTH) : label));
        }
        return new Execution(context, this);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants