Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 74ea2ad

Browse files
Palidt3chguy
Palid
and
t3chguy
authored
Fix room alias address isn't checked for validity before being shown as added (#7107)
Co-authored-by: Michael Telatynski <[email protected]>
1 parent 44d7d74 commit 74ea2ad

File tree

3 files changed

+114
-47
lines changed

3 files changed

+114
-47
lines changed

‎src/components/views/elements/RoomAliasField.tsx‎

Lines changed: 77 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ import React, { createRef, KeyboardEventHandler } from "react";
1818

1919
import { _t } from '../../../languageHandler';
2020
import withValidation from './Validation';
21-
import { MatrixClientPeg } from '../../../MatrixClientPeg';
2221
import { replaceableComponent } from "../../../utils/replaceableComponent";
2322
import Field, { IValidateOpts } from "./Field";
23+
import MatrixClientContext from "../../../contexts/MatrixClientContext";
2424

2525
interface IProps {
26-
domain: string;
26+
domain?: string;
2727
value: string;
2828
label?: string;
2929
placeholder?: string;
@@ -39,6 +39,9 @@ interface IState {
3939
// Controlled form component wrapping Field for inputting a room alias scoped to a given domain
4040
@replaceableComponent("views.elements.RoomAliasField")
4141
export default class RoomAliasField extends React.PureComponent<IProps, IState> {
42+
static contextType = MatrixClientContext;
43+
public context!: React.ContextType<typeof MatrixClientContext>;
44+
4245
private fieldRef = createRef<Field>();
4346

4447
constructor(props, context) {
@@ -50,25 +53,38 @@ export default class RoomAliasField extends React.PureComponent<IProps, IState>
5053
}
5154

5255
private asFullAlias(localpart: string): string {
53-
return `#${localpart}:${this.props.domain}`;
56+
const hashAlias = `#${ localpart }`;
57+
if (this.props.domain) {
58+
return `${hashAlias}:${this.props.domain}`;
59+
}
60+
return hashAlias;
61+
}
62+
63+
private get domainProps() {
64+
const { domain } = this.props;
65+
const prefix = <span>#</span>;
66+
const postfix = domain ? (<span title={`:${domain}`}>{ `:${domain}` }</span>) : <span />;
67+
const maxlength = domain ? 255 - domain.length - 2 : 255 - 1; // 2 for # and :
68+
const value = domain ?
69+
this.props.value.substring(1, this.props.value.length - this.props.domain.length - 1) :
70+
this.props.value.substring(1);
71+
72+
return { prefix, postfix, value, maxlength };
5473
}
5574

5675
render() {
57-
const poundSign = (<span>#</span>);
58-
const aliasPostfix = ":" + this.props.domain;
59-
const domain = (<span title={aliasPostfix}>{ aliasPostfix }</span>);
60-
const maxlength = 255 - this.props.domain.length - 2; // 2 for # and :
76+
const { prefix, postfix, value, maxlength } = this.domainProps;
6177
return (
6278
<Field
6379
label={this.props.label || _t("Room address")}
6480
className="mx_RoomAliasField"
65-
prefixComponent={poundSign}
66-
postfixComponent={domain}
81+
prefixComponent={prefix}
82+
postfixComponent={postfix}
6783
ref={this.fieldRef}
6884
onValidate={this.onValidate}
6985
placeholder={this.props.placeholder || _t("e.g. my-room")}
7086
onChange={this.onChange}
71-
value={this.props.value.substring(1, this.props.value.length - this.props.domain.length - 1)}
87+
value={value}
7288
maxLength={maxlength}
7389
disabled={this.props.disabled}
7490
autoComplete="off"
@@ -91,16 +107,58 @@ export default class RoomAliasField extends React.PureComponent<IProps, IState>
91107

92108
private validationRules = withValidation({
93109
rules: [
110+
{ key: "hasDomain",
111+
test: async ({ value }) => {
112+
// Ignore if we have passed domain
113+
if (!value || this.props.domain) {
114+
return true;
115+
}
116+
117+
if (value.split(':').length < 2) {
118+
return false;
119+
}
120+
return true;
121+
},
122+
invalid: () => _t("Missing domain separator e.g. (:domain.org)"),
123+
},
124+
{
125+
key: "hasLocalpart",
126+
test: async ({ value }) => {
127+
if (!value || this.props.domain) {
128+
return true;
129+
}
130+
131+
const split = value.split(':');
132+
if (split.length < 2) {
133+
return true; // hasDomain check will fail here instead
134+
}
135+
136+
// Define the value invalid if there's no first part (roomname)
137+
if (split[0].length < 1) {
138+
return false;
139+
}
140+
return true;
141+
},
142+
invalid: () => _t("Missing room name or separator e.g. (my-room:domain.org)"),
143+
},
94144
{
95145
key: "safeLocalpart",
96146
test: async ({ value }) => {
97147
if (!value) {
98148
return true;
99149
}
100-
const fullAlias = this.asFullAlias(value);
101-
// XXX: FIXME https://github.com/matrix-org/matrix-doc/issues/668
102-
return !value.includes("#") && !value.includes(":") && !value.includes(",") &&
103-
encodeURI(fullAlias) === fullAlias;
150+
if (!this.props.domain) {
151+
return true;
152+
} else {
153+
const fullAlias = this.asFullAlias(value);
154+
const hasColon = this.props.domain ? !value.includes(":") : true;
155+
// XXX: FIXME https://github.com/matrix-org/matrix-doc/issues/668
156+
// NOTE: We could probably use linkifyjs to parse those aliases here?
157+
return !value.includes("#") &&
158+
hasColon &&
159+
!value.includes(",") &&
160+
encodeURI(fullAlias) === fullAlias;
161+
}
104162
},
105163
invalid: () => _t("Some characters not allowed"),
106164
}, {
@@ -114,20 +172,23 @@ export default class RoomAliasField extends React.PureComponent<IProps, IState>
114172
if (!value) {
115173
return true;
116174
}
117-
const client = MatrixClientPeg.get();
175+
const client = this.context;
118176
try {
119177
await client.getRoomIdForAlias(this.asFullAlias(value));
120178
// we got a room id, so the alias is taken
121179
return false;
122180
} catch (err) {
181+
console.log(err);
123182
// any server error code will do,
124183
// either it M_NOT_FOUND or the alias is invalid somehow,
125184
// in which case we don't want to show the invalid message
126185
return !!err.errcode;
127186
}
128187
},
129188
valid: () => _t("This address is available to use"),
130-
invalid: () => _t("This address is already in use"),
189+
invalid: () => this.props.domain ?
190+
_t("This address is already in use") :
191+
_t("This address had invalid server or is already in use"),
131192
},
132193
],
133194
});

‎src/components/views/room_settings/AliasSettings.tsx‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
import React, { ChangeEvent, createRef } from "react";
17+
import React, { ChangeEvent, ContextType, createRef } from "react";
1818
import { MatrixEvent } from "matrix-js-sdk/src/models/event";
1919
import { logger } from "matrix-js-sdk/src/logger";
2020

2121
import EditableItemList from "../elements/EditableItemList";
22-
import { MatrixClientPeg } from "../../../MatrixClientPeg";
2322
import { _t } from '../../../languageHandler';
2423
import Field from "../elements/Field";
2524
import Spinner from "../elements/Spinner";
@@ -29,6 +28,7 @@ import Modal from "../../../Modal";
2928
import RoomPublishSetting from "./RoomPublishSetting";
3029
import { replaceableComponent } from "../../../utils/replaceableComponent";
3130
import RoomAliasField from "../elements/RoomAliasField";
31+
import MatrixClientContext from "../../../contexts/MatrixClientContext";
3232
import SettingsFieldset from "../settings/SettingsFieldset";
3333

3434
interface IEditableAliasesListProps {
@@ -51,11 +51,6 @@ class EditableAliasesList extends EditableItemList<IEditableAliasesListProps> {
5151
};
5252

5353
protected renderNewItemField() {
54-
// if we don't need the RoomAliasField,
55-
// we don't need to overriden version of renderNewItemField
56-
if (!this.props.domain) {
57-
return super.renderNewItemField();
58-
}
5954
const onChange = (alias) => this.onNewItemChanged({ target: { value: alias } });
6055
return (
6156
<form
@@ -98,13 +93,16 @@ interface IState {
9893

9994
@replaceableComponent("views.room_settings.AliasSettings")
10095
export default class AliasSettings extends React.Component<IProps, IState> {
96+
public static contextType = MatrixClientContext;
97+
context: ContextType<typeof MatrixClientContext>;
98+
10199
static defaultProps = {
102100
canSetAliases: false,
103101
canSetCanonicalAlias: false,
104102
};
105103

106-
constructor(props) {
107-
super(props);
104+
constructor(props, context: ContextType<typeof MatrixClientContext>) {
105+
super(props, context);
108106

109107
const state = {
110108
altAliases: [], // [ #alias:domain.tld, ... ]
@@ -138,10 +136,10 @@ export default class AliasSettings extends React.Component<IProps, IState> {
138136
private async loadLocalAliases() {
139137
this.setState({ localAliasesLoading: true });
140138
try {
141-
const cli = MatrixClientPeg.get();
139+
const mxClient = this.context;
142140
let localAliases = [];
143-
if (await cli.doesServerSupportUnstableFeature("org.matrix.msc2432")) {
144-
const response = await cli.unstableGetLocalAliases(this.props.roomId);
141+
if (await mxClient.doesServerSupportUnstableFeature("org.matrix.msc2432")) {
142+
const response = await mxClient.unstableGetLocalAliases(this.props.roomId);
145143
if (Array.isArray(response.aliases)) {
146144
localAliases = response.aliases;
147145
}
@@ -171,7 +169,7 @@ export default class AliasSettings extends React.Component<IProps, IState> {
171169

172170
if (alias) eventContent["alias"] = alias;
173171

174-
MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias",
172+
this.context.sendStateEvent(this.props.roomId, "m.room.canonical_alias",
175173
eventContent, "").catch((err) => {
176174
logger.error(err);
177175
Modal.createTrackedDialog('Error updating main address', '', ErrorDialog, {
@@ -192,7 +190,6 @@ export default class AliasSettings extends React.Component<IProps, IState> {
192190

193191
this.setState({
194192
updatingCanonicalAlias: true,
195-
altAliases,
196193
});
197194

198195
const eventContent = {};
@@ -204,19 +201,25 @@ export default class AliasSettings extends React.Component<IProps, IState> {
204201
eventContent["alt_aliases"] = altAliases;
205202
}
206203

207-
MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias",
208-
eventContent, "").catch((err) => {
209-
logger.error(err);
210-
Modal.createTrackedDialog('Error updating alternative addresses', '', ErrorDialog, {
211-
title: _t("Error updating main address"),
212-
description: _t(
213-
"There was an error updating the room's alternative addresses. " +
204+
this.context.sendStateEvent(this.props.roomId, "m.room.canonical_alias", eventContent, "")
205+
.then(() => {
206+
this.setState({
207+
altAliases,
208+
});
209+
})
210+
.catch((err) => {
211+
// TODO: Add error handling based upon server validation
212+
logger.error(err);
213+
Modal.createTrackedDialog('Error updating alternative addresses', '', ErrorDialog, {
214+
title: _t("Error updating main address"),
215+
description: _t(
216+
"There was an error updating the room's alternative addresses. " +
214217
"It may not be allowed by the server or a temporary failure occurred.",
215-
),
218+
),
219+
});
220+
}).finally(() => {
221+
this.setState({ updatingCanonicalAlias: false });
216222
});
217-
}).finally(() => {
218-
this.setState({ updatingCanonicalAlias: false });
219-
});
220223
}
221224

222225
private onNewAliasChanged = (value: string) => {
@@ -226,10 +229,10 @@ export default class AliasSettings extends React.Component<IProps, IState> {
226229
private onLocalAliasAdded = (alias: string) => {
227230
if (!alias || alias.length === 0) return; // ignore attempts to create blank aliases
228231

229-
const localDomain = MatrixClientPeg.get().getDomain();
232+
const localDomain = this.context.getDomain();
230233
if (!alias.includes(':')) alias += ':' + localDomain;
231234

232-
MatrixClientPeg.get().createAlias(alias, this.props.roomId).then(() => {
235+
this.context.createAlias(alias, this.props.roomId).then(() => {
233236
this.setState({
234237
localAliases: this.state.localAliases.concat(alias),
235238
newAlias: null,
@@ -253,7 +256,7 @@ export default class AliasSettings extends React.Component<IProps, IState> {
253256
const alias = this.state.localAliases[index];
254257
// TODO: In future, we should probably be making sure that the alias actually belongs
255258
// to this room. See https://github.com/vector-im/element-web/issues/7353
256-
MatrixClientPeg.get().deleteAlias(alias).then(() => {
259+
this.context.deleteAlias(alias).then(() => {
257260
const localAliases = this.state.localAliases.filter(a => a !== alias);
258261
this.setState({ localAliases });
259262

@@ -322,9 +325,9 @@ export default class AliasSettings extends React.Component<IProps, IState> {
322325
}
323326

324327
render() {
325-
const cli = MatrixClientPeg.get();
326-
const localDomain = cli.getDomain();
327-
const isSpaceRoom = cli.getRoom(this.props.roomId)?.isSpaceRoom();
328+
const mxClient = this.context;
329+
const localDomain = mxClient.getDomain();
330+
const isSpaceRoom = mxClient.getRoom(this.props.roomId)?.isSpaceRoom();
328331

329332
let found = false;
330333
const canonicalValue = this.state.canonicalAlias || "";

‎src/i18n/strings/en_EN.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,10 +2280,13 @@
22802280
"In reply to <a>this message</a>": "In reply to <a>this message</a>",
22812281
"Room address": "Room address",
22822282
"e.g. my-room": "e.g. my-room",
2283+
"Missing domain separator e.g. (:domain.org)": "Missing domain separator e.g. (:domain.org)",
2284+
"Missing room name or separator e.g. (my-room:domain.org)": "Missing room name or separator e.g. (my-room:domain.org)",
22832285
"Some characters not allowed": "Some characters not allowed",
22842286
"Please provide an address": "Please provide an address",
22852287
"This address is available to use": "This address is available to use",
22862288
"This address is already in use": "This address is already in use",
2289+
"This address had invalid server or is already in use": "This address had invalid server or is already in use",
22872290
"Server Options": "Server Options",
22882291
"You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use Element with an existing Matrix account on a different homeserver.": "You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use Element with an existing Matrix account on a different homeserver.",
22892292
"Join millions for free on the largest public server": "Join millions for free on the largest public server",

0 commit comments

Comments
 (0)