Skip to content

Refactor string.substring usages to account for surrogate pair characters #7206

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.

Sign up for GitHub

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

Merged
XingY merged 4 commits into from
Nov 18, 2025
Merged

Refactor string.substring usages to account for surrogate pair characters #7206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdb4b7f
Handle surrogate pair characters substring
XingY Nov 13, 2025
bb0eb4a
CRLF
XingY Nov 13, 2025
4273559
fix
XingY Nov 13, 2025
e5cc3ef
Fix overlapping sheet names
labkey-tchad Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/Container.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.labkey.api.util.NetworkDrive;
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.util.Path;
import org.labkey.api.util.StringUtilsLabKey;
Copy link
Contributor

@labkey-nicka labkey-nicka Nov 18, 2025

Choose a reason for hiding this comment

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

There is this line in PageFlowUtil. Does it need to use these methods?

if (ext != null && mimeType != null)
{
    fileName = fileName.substring(0, fileName.length() - (ext.length() + 1)) + mimeType.getExtension();
}

Copy link
Contributor Author

@XingY XingY Nov 18, 2025

Choose a reason for hiding this comment

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

I don't think this one needs it. ext and mimeType both are unlikely to have special characters, so the endInd should be safe.

import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.ActionURL;
import org.labkey.api.view.FolderTab;
Expand Down Expand Up @@ -1594,7 +1595,7 @@ public String getContainerNoun(boolean titleCase)
String noun = _containerType.getContainerNoun(this);
if (titleCase)
{
return noun.substring(0, 1).toUpperCase() + noun.substring(1);
return StringUtilsLabKey.leftSurrogatePairFriendly(noun, 1).toUpperCase() + StringUtilsLabKey.rightSurrogatePairFriendly(noun, noun.length() - 1);
Copy link
Contributor

@labkey-nicka labkey-nicka Nov 18, 2025

Choose a reason for hiding this comment

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

In general, how should someone know if they should use substring() versus using one of these utility methods? For example, I still see a lot of usages of .substring(0, some of which seem eligible for switching out for these utilities.

Copy link
Contributor Author

@XingY XingY Nov 18, 2025

Choose a reason for hiding this comment

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

Like discussed, I think generally a static constant endIndex is a good candidate for using those new methods. When paired with indexOf... as end, then probably substring is better. But each case needs to be evaluated separately to prioritize overflow/accuracy, etc.

}

return noun;
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/ExcelCellUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.apache.poi.ss.usermodel.CellStyle;
import org.apache.poi.ss.usermodel.Workbook;
import org.labkey.api.util.DateUtil;
import org.labkey.api.util.StringUtilsLabKey;

import java.io.File;
import java.math.BigDecimal;
Expand Down Expand Up @@ -188,7 +189,7 @@ public static void writeCell(Cell cell, CellStyle style, int simpleType, String
// Check if the string is too long
if (s.length() > 32767)
{
s = s.substring(0, 32762) + "...";
s = StringUtilsLabKey.leftSurrogatePairFriendly(s, 32762) + "...";
}
// Ensure the row is tall enough to show the full values when there are newlines
int newlines = StringUtils.countMatches(s, '\n');
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/ExcelWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.poi.ss.usermodel.Workbook;
import org.apache.poi.ss.util.CellRangeAddress;
import org.apache.poi.ss.util.WorkbookUtil;
import org.labkey.api.util.StringUtilsLabKey;
import org.apache.poi.xssf.streaming.SXSSFSheet;
import org.apache.poi.xssf.streaming.SXSSFWorkbook;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -375,7 +376,7 @@ public void setSheetName(String sheetName)
sheetName = "Sheet";

if (sheetName.length() > 31)
return cleanSheetName(sheetName.substring(0, 31));
return cleanSheetName(StringUtilsLabKey.leftSurrogatePairFriendly(sheetName, 31));

return WorkbookUtil.createSafeSheetName(sheetName, '_');
}
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/SQLFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.labkey.api.util.GUID;
import org.labkey.api.util.JdbcUtil;
import org.labkey.api.util.Pair;
import org.labkey.api.util.StringUtilsLabKey;

import java.math.BigDecimal;
import java.math.BigInteger;
Expand Down Expand Up @@ -754,7 +755,7 @@ public boolean appendComment(String comment, SqlDialect dialect)
sb.append("\n-- ");
boolean truncated = comment.length() > 1000;
if (truncated)
comment = comment.substring(0,1000);
comment = StringUtilsLabKey.leftSurrogatePairFriendly(comment, 1000);
sb.append(comment);
if (StringUtils.countMatches(comment, "'")%2==1)
sb.append("'");
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/TSVWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.junit.Assert;
import org.junit.Test;
import org.labkey.api.util.FileUtil;
import org.labkey.api.util.StringUtilsLabKey;

import java.io.IOException;
import java.io.PrintWriter;
Expand Down Expand Up @@ -109,7 +110,7 @@ public void setFilenamePrefix(String filenamePrefix)
_filenamePrefix = badChars.matcher(filenamePrefix).replaceAll("_");

if (_filenamePrefix.length() > 30)
_filenamePrefix = _filenamePrefix.substring(0, 30);
_filenamePrefix = StringUtilsLabKey.leftSurrogatePairFriendly(_filenamePrefix, 30);
}

public void setDelimiterCharacter(char delimiter)
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/dialect/StatementWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.labkey.api.util.DebugInfoDumper;
import org.labkey.api.util.ExceptionUtil;
import org.labkey.api.util.MemTracker;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.view.ViewServlet;

import java.io.InputStream;
Expand Down Expand Up @@ -2876,7 +2877,7 @@ else if (o instanceof String)
else
value = String.valueOf(o);
if (value.length() > 100)
value = value.substring(0, 100) + ". . .";
value = StringUtilsLabKey.leftSurrogatePairFriendly(value, 100) + ". . .";
Copy link
Contributor

@labkey-nicka labkey-nicka Nov 18, 2025

Choose a reason for hiding this comment

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

nit: This isn't a part of these changes but the names of these utility functions does not describe what they do. Consider a rename refactor to something like leftSurrogateSafeTruncate() and rightSurrogateSafeTruncate().

logEntry.append("\n --[").append(i).append("] ");
logEntry.append(value);
Class<?> c = null==o ? null : o.getClass();
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/exp/OntologyManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.labkey.api.util.HtmlString;
import org.labkey.api.util.HtmlStringBuilder;
import org.labkey.api.util.Pair;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.ResultSetUtil;
import org.labkey.api.util.TestContext;
import org.labkey.api.view.HttpView;
Expand Down Expand Up @@ -767,7 +768,7 @@ public static boolean validateProperty(List<? extends IPropertyValidator> valida
int stringLength = value == null ? 0 : value.toString().length();
if (value != null && prop.isStringType() && stringLength > stringLengthLimit)
{
String s = stringLength < 100 ? value.toString() : value.toString().substring(0, 100);
String s = stringLength <= 100 ? value.toString() : StringUtilsLabKey.leftSurrogatePairFriendly(value.toString(), 100);
errors.add(new PropertyValidationError("Field '" + prop.getName() + "' is limited to " + stringLengthLimit + " characters, but the value is " + stringLength + " characters. (The value starts with '" + s + "...')", prop.getName()));
ret = false;
}
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/jsp/LabKeyJspWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.labkey.api.util.DOM;
import org.labkey.api.util.HelpTopic;
import org.labkey.api.util.SafeToRender;
import org.labkey.api.util.StringUtilsLabKey;

import jakarta.servlet.jsp.JspWriter;
import java.io.IOException;
Expand All @@ -44,7 +45,7 @@ private String truncateAndQuote(String s)
{
return null;
}
return "'" + (s.length() < 50 ? s : (s.substring(0, 50) + "...")) + "'";
return "'" + (s.length() < 50 ? s : (StringUtilsLabKey.leftSurrogatePairFriendly(s, 50) + "...")) + "'";
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/util/MemTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public String getClassName()
public String getObjectSummary()
{
String desc = getObjectDescription();
return desc.length() > 50 ? desc.substring(0, 50) + "..." : desc;
return desc.length() > 50 ? StringUtilsLabKey.leftSurrogatePairFriendly(desc, 50) + "..." : desc;
}

public boolean hasShortSummary()
Expand Down
4 changes: 2 additions & 2 deletions core/src/org/labkey/core/admin/AdminController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9796,7 +9796,7 @@ public void validateCommand(TabActionForm form, Errors errors)
}

if (name.length() > 50)
name = name.substring(0, 50).trim();
name = StringUtilsLabKey.leftSurrogatePairFriendly(name, 50).trim();

CaseInsensitiveHashMap<Portal.PortalPage> pages = new CaseInsensitiveHashMap<>(Portal.getPages(tabContainer, true));
CaseInsensitiveHashMap<FolderTab> folderTabMap = new CaseInsensitiveHashMap<>();
Expand Down Expand Up @@ -9850,7 +9850,7 @@ public ApiResponse execute(TabActionForm form, BindException errors)
// The name, which shows up on the url, is trimmed to 50 characters. The caption, which is derived from the
// name, and is editable, is allowed to be 64 characters.
if (name.length() > 50)
name = name.substring(0, 50).trim();
name = StringUtilsLabKey.leftSurrogatePairFriendly(name, 50).trim();

Portal.saveParts(container, name);
Portal.addProperty(container, name, Portal.PROP_CUSTOMTAB);
Expand Down
3 changes: 2 additions & 1 deletion experiment/src/org/labkey/experiment/api/AbstractRunInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.jetbrains.annotations.Nullable;
import org.labkey.api.exp.IdentifiableBase;
import org.labkey.api.exp.Lsid;
import org.labkey.api.util.StringUtilsLabKey;

import java.util.Objects;

Expand Down Expand Up @@ -65,7 +66,7 @@ public void setRole(@Nullable String role)
// so truncate here if needed to prevent a SQLException later
if (role.length() > 50)
{
role = role.substring(0, 49);
role = StringUtilsLabKey.leftSurrogatePairFriendly(role, 49);
}
_role = role;
}
Expand Down
3 changes: 2 additions & 1 deletion experiment/src/org/labkey/experiment/api/ExpDataImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.labkey.api.util.NetworkDrive;
import org.labkey.api.util.Pair;
import org.labkey.api.util.Path;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.InputBuilder;
import org.labkey.api.view.ActionURL;
Expand Down Expand Up @@ -796,7 +797,7 @@ private static void appendTokens(StringBuilder sb, Collection<String> toks)
if (toks.isEmpty())
return;

sb.append(toks.stream().map(s -> s.length() > 30 ? s.substring(0, 30) + "\u2026" : s).collect(Collectors.joining(", "))).append("\n");
sb.append(toks.stream().map(s -> s.length() > 30 ? StringUtilsLabKey.leftSurrogatePairFriendly(s, 30) + "\u2026" : s).collect(Collectors.joining(", "))).append("\n");
}

private static class ExpDataResource extends SimpleDocumentResource
Expand Down
7 changes: 4 additions & 3 deletions mothership/src/org/labkey/mothership/MothershipManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.labkey.api.util.JsonUtil;
import org.labkey.api.util.MothershipReport;
import org.labkey.api.util.ReentrantLockWithName;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.logging.LogHelper;

import java.io.IOException;
Expand Down Expand Up @@ -109,19 +110,19 @@ public void insertException(ExceptionStackTrace stackTrace, ExceptionReport repo

String url = report.getUrl();
if (null != url && url.length() > 512)
report.setURL(url.substring(0, 506) + "...");
report.setURL(StringUtilsLabKey.leftSurrogatePairFriendly(url, 506) + "...");

String referrerURL = report.getReferrerURL();
if (null != referrerURL && referrerURL.length() > 512)
report.setReferrerURL(referrerURL.substring(0, 506) + "...");
report.setReferrerURL(StringUtilsLabKey.leftSurrogatePairFriendly(referrerURL, 506) + "...");

String browser = report.getBrowser();
if (null != browser && browser.length() > 100)
report.setBrowser(browser.substring(0,90) + "...");

String exceptionMessage = report.getExceptionMessage();
if (null != exceptionMessage && exceptionMessage.length() > 1000)
report.setExceptionMessage(exceptionMessage.substring(0,990) + "...");
report.setExceptionMessage(StringUtilsLabKey.leftSurrogatePairFriendly(exceptionMessage, 990) + "...");

String actionName = report.getPageflowAction();
if (null != actionName && actionName.length() > 40)
Expand Down
2 changes: 1 addition & 1 deletion pipeline/src/org/labkey/pipeline/api/WorkDirectoryRemote.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public WorkDirectory createWorkDirectory(String jobId, FileAnalysisJobSupport su
// Don't let the total path get too long - Windows doesn't like paths longer than 255 characters
// so if there's a ridiculously long file name, we don't want to duplicate its name in the
// directory too
name = name.substring(0, 9);
name = StringUtilsLabKey.leftSurrogatePairFriendly(name, 9);
}
else if (name.length() < 3)
{
Expand Down
3 changes: 2 additions & 1 deletion query/src/org/labkey/query/controllers/QueryController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@
import org.labkey.api.util.ResponseHelper;
import org.labkey.api.util.ReturnURLString;
import org.labkey.api.util.StringExpression;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.TestContext;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.UnexpectedException;
Expand Down Expand Up @@ -2134,7 +2135,7 @@ public Object execute(ExportQueriesForm form, BindException errors) throws Excep
throw new IllegalArgumentException("Cannot create sheet names from overlapping query names.");
for (int i = 0; i < queryForms.size(); i++)
{
sheetNames.put(entry.getValue().get(i), name.substring(0, name.length() - countLength) + "(" + i + ")");
sheetNames.put(entry.getValue().get(i), StringUtilsLabKey.leftSurrogatePairFriendly(name, name.length() - countLength) + "(" + i + ")");
}
}
else
Expand Down
3 changes: 2 additions & 1 deletion search/src/org/labkey/search/SearchController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.labkey.api.util.HtmlStringBuilder;
import org.labkey.api.util.Path;
import org.labkey.api.util.ResponseHelper;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.ActionURL;
Expand Down Expand Up @@ -1126,7 +1127,7 @@ public static void audit(@Nullable User user, @Nullable Container c, String quer
c = ContainerManager.getRoot();

if (query.length() > 200)
query = query.substring(0, 197) + "...";
query = StringUtilsLabKey.leftSurrogatePairFriendly(query, 197) + "...";

SearchAuditProvider.SearchAuditEvent event = new SearchAuditProvider.SearchAuditEvent(c, comment);
event.setQuery(query);
Expand Down
Loading