Skip to content

Commit d89f53d

Browse files
committed
Address review nits: rune-count fleet names, drop dead validators, rename constant
- Switch fleet-name length checks in ee/server/service/teams.go to utf8.RuneCountInString so multibyte names within the character cap are accepted (previously len() over-counted bytes and could reject valid names). - Drop the no-op description validator from both Label form helpers. - Rename MAX_ENTITY_NAME_LENGTH to MAX_ENTITY_CHAR_LENGTH and broaden the docstring to cover description fields too.
1 parent 3ce51d8 commit d89f53d

25 files changed

Lines changed: 73 additions & 72 deletions

File tree

ee/server/service/teams.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/http"
1111
"net/url"
1212
"strings"
13+
"unicode/utf8"
1314

1415
"golang.org/x/text/unicode/norm"
1516

@@ -89,7 +90,7 @@ func (svc *Service) NewTeam(ctx context.Context, p fleet.TeamPayload) (*fleet.Te
8990
if *p.Name == "" {
9091
return nil, fleet.NewInvalidArgumentError("name", "may not be empty")
9192
}
92-
if len(*p.Name) > fleet.MaxTeamNameLength {
93+
if utf8.RuneCountInString(*p.Name) > fleet.MaxTeamNameLength {
9394
return nil, fleet.NewInvalidArgumentError("name", fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength))
9495
}
9596
if fleet.IsReservedTeamName(*p.Name) {
@@ -168,7 +169,7 @@ func (svc *Service) ModifyTeam(ctx context.Context, teamID uint, payload fleet.T
168169
if *payload.Name == "" {
169170
return nil, fleet.NewInvalidArgumentError("name", "may not be empty")
170171
}
171-
if len(*payload.Name) > fleet.MaxTeamNameLength {
172+
if utf8.RuneCountInString(*payload.Name) > fleet.MaxTeamNameLength {
172173
return nil, fleet.NewInvalidArgumentError("name", fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength))
173174
}
174175
if fleet.IsReservedTeamName(*payload.Name) {
@@ -1270,7 +1271,7 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec,
12701271
if spec.Name == "" {
12711272
return nil, fleet.NewInvalidArgumentError("name", "name may not be empty")
12721273
}
1273-
if len(spec.Name) > fleet.MaxTeamNameLength {
1274+
if utf8.RuneCountInString(spec.Name) > fleet.MaxTeamNameLength {
12741275
return nil, fleet.NewInvalidArgumentError("name", fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength))
12751276
}
12761277
if fleet.IsReservedTeamName(spec.Name) {

ee/server/service/teams_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ func TestNewTeamNameValidation(t *testing.T) {
145145
teamName: new(strings.Repeat("a", fleet.MaxTeamNameLength+1)),
146146
wantErr: fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength),
147147
},
148+
{
149+
// Guards against regressing to byte-based length checks, which
150+
// would reject multibyte names that fit within the character cap.
151+
name: "multibyte name at max character length is accepted",
152+
teamName: new(strings.Repeat("日", fleet.MaxTeamNameLength)),
153+
wantName: strings.Repeat("日", fleet.MaxTeamNameLength),
154+
},
148155
}
149156

150157
for _, tc := range testCases {
@@ -278,6 +285,11 @@ func TestModifyTeamNameValidation(t *testing.T) {
278285
teamName: new(strings.Repeat("a", fleet.MaxTeamNameLength+1)),
279286
wantErr: fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength),
280287
},
288+
{
289+
name: "multibyte name at max character length is accepted",
290+
teamName: new(strings.Repeat("日", fleet.MaxTeamNameLength)),
291+
wantName: strings.Repeat("日", fleet.MaxTeamNameLength),
292+
},
281293
}
282294

283295
for _, tc := range testCases {
@@ -398,6 +410,11 @@ func TestApplyTeamSpecsNameValidation(t *testing.T) {
398410
teamName: strings.Repeat("a", fleet.MaxTeamNameLength+1),
399411
wantErr: fmt.Sprintf("may not exceed %d characters", fleet.MaxTeamNameLength),
400412
},
413+
{
414+
name: "multibyte name at max character length is accepted",
415+
teamName: strings.Repeat("日", fleet.MaxTeamNameLength),
416+
wantName: strings.Repeat("日", fleet.MaxTeamNameLength),
417+
},
401418
}
402419

403420
for _, tc := range testCases {

frontend/components/forms/packs/EditPackForm/EditPackForm.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useState } from "react";
22
import useDeepEffect from "hooks/useDeepEffect";
33

4-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
4+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
55
import Button from "components/buttons/Button";
66

77
import { IQuery } from "interfaces/query";
@@ -112,7 +112,7 @@ const EditPackForm = ({
112112
name="name"
113113
error={errors.name}
114114
inputWrapperClass={`${baseClass}__pack-title`}
115-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
115+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
116116
/>
117117
<InputField
118118
onChange={onChangePackDescription}
@@ -122,7 +122,7 @@ const EditPackForm = ({
122122
name="description"
123123
placeholder="Add a description of your pack"
124124
type="textarea"
125-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
125+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
126126
/>
127127
<SelectTargetsDropdown
128128
label="Select pack targets"

frontend/components/forms/packs/NewPackForm/NewPackForm.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { IQuery } from "interfaces/query";
66
import { ITarget, ITargetsAPIResponse } from "interfaces/target";
77
import { IEditPackFormData } from "interfaces/pack";
88
import PATHS from "router/paths";
9-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
9+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
1010

1111
import InputField from "components/forms/fields/InputField";
1212
import BackButton from "components/BackButton";
@@ -94,7 +94,7 @@ const NewPackForm = ({
9494
error={errors.name}
9595
inputWrapperClass={`${baseClass}__pack-title`}
9696
autofocus
97-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
97+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
9898
/>
9999
<InputField
100100
onChange={onChangePackDescription}
@@ -104,7 +104,7 @@ const NewPackForm = ({
104104
name="description"
105105
placeholder="Add a description of your pack"
106106
type="textarea"
107-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
107+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
108108
/>
109109
<SelectTargetsDropdown
110110
label="Select pack targets"

frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { SingleValue } from "react-select-5";
44

55
import {
66
DEFAULT_USE_QUERY_OPTIONS,
7-
MAX_ENTITY_NAME_LENGTH,
7+
MAX_ENTITY_CHAR_LENGTH,
88
} from "utilities/constants";
99

1010
import paths from "router/paths";
@@ -206,7 +206,7 @@ const AddCertModal = ({
206206
helpText="Letters, numbers, spaces, dashes, and underscores only. Name can be used as certificate alias to reference in configuration profiles."
207207
parseTarget
208208
placeholder="VPN certificate"
209-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
209+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
210210
/>
211211
<InputField
212212
name="subjectName"

frontend/pages/ManageControlsPage/Variables/components/AddCustomVariableModal/AddCustomVariableModal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { hasStatusKey, getErrorReason } from "interfaces/errors";
66
import variablesAPI from "services/entities/variables";
77
import {
88
LEARN_MORE_ABOUT_BASE_LINK,
9-
MAX_ENTITY_NAME_LENGTH,
9+
MAX_ENTITY_CHAR_LENGTH,
1010
} from "utilities/constants";
1111
import { notify } from "components/ToastNotification";
1212
import CustomLink from "components/CustomLink";
@@ -120,7 +120,7 @@ const AddCustomVariableModal = ({
120120
</span>
121121
}
122122
error={formValidation.name?.message}
123-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
123+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
124124
/>
125125
<InputField
126126
onChange={onInputChange}

frontend/pages/SoftwarePage/SoftwareLibrary/SelfServiceCategoriesPage/AddCategoryModal/AddCategoryModal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, { useState } from "react";
22

33
import selfServiceCategoriesAPI from "services/entities/self_service_categories";
44
import { hasStatusKey } from "interfaces/errors";
5-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
5+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
66

77
import Button from "components/buttons/Button";
88
import InputField from "components/forms/fields/InputField";
@@ -72,7 +72,7 @@ const AddCategoryModal = ({
7272
error={error}
7373
autofocus
7474
ignore1password
75-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
75+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
7676
/>
7777
<div className="modal-cta-wrap">
7878
<Button type="submit" disabled={isDisabled} isLoading={isSubmitting}>

frontend/pages/SoftwarePage/SoftwareLibrary/SelfServiceCategoriesPage/EditCategoryModal/EditCategoryModal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import React, { useState } from "react";
33
import selfServiceCategoriesAPI from "services/entities/self_service_categories";
44
import { hasStatusKey } from "interfaces/errors";
55
import { ISelfServiceCategory } from "interfaces/self_service_category";
6-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
6+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
77

88
import Button from "components/buttons/Button";
99
import InputField from "components/forms/fields/InputField";
@@ -73,7 +73,7 @@ const EditCategoryModal = ({
7373
error={error}
7474
autofocus
7575
ignore1password
76-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
76+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
7777
/>
7878
<div className="modal-cta-wrap">
7979
<Button type="submit" disabled={isDisabled} isLoading={isSubmitting}>

frontend/pages/admin/IntegrationsPage/cards/CertificateAuthorities/components/CustomESTForm/CustomESTForm.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useMemo } from "react";
22

33
import { ICertificateAuthorityPartial } from "interfaces/certificates";
4-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
4+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
55

66
import InputField from "components/forms/fields/InputField";
77
import Button from "components/buttons/Button";
@@ -64,7 +64,7 @@ const CustomESTForm = ({
6464
parseTarget
6565
placeholder="WIFI_CERTIFICATE"
6666
helpText="Letters, numbers, and underscores only."
67-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
67+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
6868
/>
6969
<InputField
7070
label="URL"

frontend/pages/admin/IntegrationsPage/cards/CertificateAuthorities/components/CustomSCEPForm/CustomSCEPForm.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React, { useMemo, useState } from "react";
22

33
import { ICertificateAuthorityPartial } from "interfaces/certificates";
4-
import { MAX_ENTITY_NAME_LENGTH } from "utilities/constants";
4+
import { MAX_ENTITY_CHAR_LENGTH } from "utilities/constants";
55

66
import InputField from "components/forms/fields/InputField";
77
import Button from "components/buttons/Button";
@@ -81,7 +81,7 @@ const CustomSCEPForm = ({
8181
parseTarget
8282
placeholder="WIFI_CERTIFICATE"
8383
helpText="Letters, numbers, and underscores only. Fleet will create configuration profile variables with the name as suffix (e.g. $FLEET_VAR_CUSTOM_SCEP_CHALLENGE_WIFI_CERTIFICATE)."
84-
inputOptions={{ maxLength: MAX_ENTITY_NAME_LENGTH }}
84+
inputOptions={{ maxLength: MAX_ENTITY_CHAR_LENGTH }}
8585
/>
8686
<InputField
8787
label="SCEP URL"

0 commit comments

Comments
 (0)