Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

fix: provide default port when adding custom network #1510

Merged
merged 3 commits into from
Oct 14, 2019
Merged

fix: provide default port when adding custom network #1510

merged 3 commits into from
Oct 14, 2019

Conversation

dated
Copy link
Contributor

@dated dated commented Oct 11, 2019

Summary

Adding a custom network fails if the seed server has no port. This PR provides default values if the port is missing.

Regarding the changes made to:

__parseCurrentPeer () {
    const matches = /(https?:\/\/)([a-zA-Z0-9.-_]+):([0-9]+)/.exec(this.host)
    const scheme = matches[1]
    const ip = matches[2]
    const isHttps = scheme === 'https://'
    let port = isHttps ? 443 : 80
    if (matches[3]) {
      port = matches[3]
    }

    return {
      ip,
      port,
      isHttps
    }

If this.host contains no port, matches is null and const scheme = matches[1] throws a type error. Checking against matches[3] afterwards is useless at this point.

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@ghost
Copy link

ghost commented Oct 11, 2019

Thanks for submitting this pull request! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@ghost ghost added Complexity: Low Less than 64 lines changed. Type: Bugfix The pull request fixes an incorrect functionality or behaviour. labels Oct 11, 2019
@luciorubeens luciorubeens added the Bounty: Tier 4 Awarded for small features, refactorings, improvements. This is valued at 20 USD. label Oct 14, 2019
@ghost
Copy link

ghost commented Oct 14, 2019

A member has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait.

Thank you for your contribution!

@ghost ghost added the Status: Member Approved The pull request has been approved by a member. label Oct 14, 2019
@luciorubeens luciorubeens merged commit 5340a9f into ArkEcosystem:develop Oct 14, 2019
@ghost
Copy link

ghost commented Oct 14, 2019

Your pull request has been merged and marked as tier 4. It will earn you $20 USD.

@dated dated deleted the fix/network-seed-validation branch October 14, 2019 10:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bounty: Tier 4 Awarded for small features, refactorings, improvements. This is valued at 20 USD. Complexity: Low Less than 64 lines changed. Status: Member Approved The pull request has been approved by a member. Type: Bugfix The pull request fixes an incorrect functionality or behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants