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.

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

Jump to bottom

fix issue#59: add validation check during submission on WEB UI #71

Merged
merged 1 commit into from
Jun 28, 2017
Merged

fix issue#59: add validation check during submission on WEB UI #71

merged 1 commit into from
Jun 28, 2017

Conversation

LeZhang2016
Copy link

No description provided.

bukepo
bukepo reviewed Jun 26, 2017
View reviewed changes
src/web/web-service/frontend/index.html Outdated
<div ng-messages="threadForm.networkName.$error">
<div ng-message="required">This is required.</div>
<div ng-message="md-maxlength">The Netowork Name must be less than 16 characters long.</div>
Copy link
Member

Choose a reason for hiding this comment

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

The Netowork Name must be no more than 16 characters!?

Copy link
Member

Choose a reason for hiding this comment

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

"less than" or "no more than"?

src/web/web-service/frontend/index.html Outdated
</div>
</md-input-container>
</div>

<div layout="row">
<md-input-container flex="100">
<label>On-Mesh Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1})/g">
Copy link
Member

Choose a reason for hiding this comment

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

Does this regex allows prefix length?
Is link-local address allowed as Thread prefix?

@LeZhang2016 LeZhang2016 force-pushed the issue#59 branch 2 times, most recently from 3e82653 to 3a05140 Compare June 27, 2017 01:36
bukepo
bukepo reviewed Jun 27, 2017
View reviewed changes
src/web/web-service/frontend/index.html Outdated
</div>
</md-input-container>
</div>

<div layout="row">
<md-input-container flex="50">
<label>PAN ID</label>
<input required name="panId" ng-model="thread.panId">
<input required name="panId" ng-model="thread.panId" ng-pattern="/^0x[0-9a-fA-F]{4}/g">
Copy link
Member

Choose a reason for hiding this comment

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

not sure if this regex allows 0x1234567890?

src/web/web-service/frontend/index.html Outdated
@@ -199,27 +201,31 @@ <h4 style="margin: 18px 12px 12px 24px">OT Border Router</h4>
<div layout="row">
<md-input-container flex="50">
<label>Network Key</label>
<input required name="networkKey" ng-model="thread.networkKey">
<input required minlength="32" maxlength="32" ng-pattern="/^[0-9a-fA-F]{32}/g" name="networkKey" ng-model="thread.networkKey">
Copy link
Member

Choose a reason for hiding this comment

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

same as above

src/web/web-service/frontend/index.html Outdated
</div>
</md-input-container>
</div>

<div layout="row">
<md-input-container flex="100">
<label>On-Mesh Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^((?!(fe80:)|(ff[01][0-9]:))([0-9a-fA-F]{1,4}:)(([0-9a-fA-F]{1,4}:){1,7}):((/[1-9][0-9]?){0,1}))$/">
Copy link
Member

Choose a reason for hiding this comment

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

does this allows ff05::/64

src/web/web-service/frontend/res/js/app.js Outdated
@@ -212,7 +217,12 @@
};


$scope.showConfirm = function(ev) {
$scope.showConfirm = function(ev, valid) {
if (valid === false)
Copy link
Member

Choose a reason for hiding this comment

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

if valid is boolean, is it simpler to use !valid here?

src/web/web-service/frontend/join.dialog.html Outdated
</div>
</md-input-container>
</div>
<div layout="row">
<md-input-container flex="80">
<label>Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^((?!(fe80:)|(ff[01][0-9]:))([0-9a-fA-F]{1,4}:)(([0-9a-fA-F]{1,4}:){1,7}):((/[1-9][0-9]?){0,1}))$/">
Copy link
Member

Choose a reason for hiding this comment

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

same as above

@LeZhang2016 LeZhang2016 force-pushed the issue#59 branch 3 times, most recently from 50d532f to aabead4 Compare June 27, 2017 03:24
@jwhui jwhui requested a review from bukepo June 27, 2017 16:01
bukepo
bukepo reviewed Jun 28, 2017
View reviewed changes
src/web/web-service/frontend/index.html Outdated
</div>
</md-input-container>
</div>

<div layout="row">
<md-input-container flex="100">
<label>On-Mesh Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^((?!(fe80:)|(ff[0-9a-fA-F][0-9a-fA-F]:))([0-9a-fA-F]{1,4}:)(([0-9a-fA-F]{1,4}:){1,7}):((/[1-9][0-9]?){0,1}))$/">
Copy link
Member

@bukepo bukepo Jun 28, 2017 •
edited

Choose a reason for hiding this comment

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

It seems this regex cannot work. Have you tried the following IPs?

FF01:0:0:0::/64
FE80:200:5aee::/64
fd11:22::/120
abcd::/64
2::/64
fd::1:0/120

I guess we can add regex modifier to perform case-insensitive matching. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions

bukepo
bukepo reviewed Jun 28, 2017
View reviewed changes
src/web/web-service/frontend/join.dialog.html Outdated
</div>
</md-input-container>
</div>
<div layout="row">
<md-input-container flex="80">
<label>Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^((?!(fe80:)|(ff[0-9a-fA-F][0-9a-fA-F]:))([0-9a-fA-F]{1,4}:)(([0-9a-fA-F]{1,4}:){1,7}):((/[1-9][0-9]?){0,1}))$/">
Copy link
Member

Choose a reason for hiding this comment

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

I think the / separator for prefix should be escaped.

@LeZhang2016 LeZhang2016 force-pushed the issue#59 branch 2 times, most recently from 1479f74 to 982a925 Compare June 28, 2017 04:18
LeZhang2016
LeZhang2016 commented Jun 28, 2017
View reviewed changes
src/web/web-service/frontend/join.dialog.html Outdated
</div>
</md-input-container>
</div>
<div layout="row">
<md-input-container flex="80">
<label>Prefix</label>
<input required name="prefix" ng-model="thread.prefix">
<input required name="prefix" ng-model="thread.prefix" ng-pattern="/^(?!([fF][eE]80:|[fF]{2}[0-9a-fA-F][0-9a-fA-F]:))(\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*)(/(1([01][0-9]|2[0-8]))|/([0-9][0-9]?))?$/">
<div ng-messages="threadForm.prefix.$error">
Copy link
Author

Choose a reason for hiding this comment

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

the regular expression is composed by three parts:

  1. ?!([fF][eE]80:|[fF]{2}[0-9a-fA-F][0-9a-fA-F]:)) filters out link local address and multicast address.
  2. (\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:)))(%.+)?\s*) checks valid IPv6 address.
  3. (/(1([01][0-9]|2[0-8]))|/([0-9][0-9]?))? checks the prefix length [0, 128].

Copy link
Member

Choose a reason for hiding this comment

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

Should exclude :: as well.

@jwhui jwhui merged commit 5069707 into openthread:master Jun 28, 2017
@LeZhang2016 LeZhang2016 mentioned this pull request Jul 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bukepo bukepo bukepo approved these changes

@jwhui jwhui jwhui approved these changes

@lanyuwen lanyuwen Awaiting requested review from lanyuwen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@LeZhang2016 @bukepo @lanyuwen @jwhui @ZhangLe2016