Skip to content

Changes to files that had linting issue#3731

Merged
melekes merged 1 commit intotendermint:developfrom
tac0turtle:marko/linting-cleanup
Jun 21, 2019
Merged

Changes to files that had linting issue#3731
melekes merged 1 commit intotendermint:developfrom
tac0turtle:marko/linting-cleanup

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Jun 19, 2019

  • Govet issues fixed
  • 1 gosec issue solved using nolint

Refs #3262

Signed-off-by: Marko Baricevic marbar3778@yahoo.com

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

- Govet issues fixed
- 1 gosec issue solved using nolint

Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
@codecov-io
Copy link

Codecov Report

Merging #3731 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           develop   #3731      +/-   ##
==========================================
+ Coverage    63.87%   63.9%   +0.02%     
==========================================
  Files          241     241              
  Lines        19985   19978       -7     
==========================================
+ Hits         12766   12767       +1     
+ Misses        6171    6166       -5     
+ Partials      1048    1045       -3
Impacted Files Coverage Δ
consensus/reactor.go 72.58% <ø> (+0.28%) ⬆️
types/block.go 72.23% <100%> (ø) ⬆️
types/tx.go 86.95% <0%> (-4.35%) ⬇️
blockchain/reactor.go 70.56% <0%> (-0.94%) ⬇️
node/node.go 63.49% <0%> (-0.32%) ⬇️
blockchain/pool.go 80.92% <0%> (+0.65%) ⬆️
privval/signer_service_endpoint.go 89.09% <0%> (+3.63%) ⬆️
libs/events/events.go 98.05% <0%> (+4.85%) ⬆️


func getServiceURL(rootURL string) (url, urnDomain string, err error) {
r, err := http.Get(rootURL)
r, err := http.Get(rootURL) // nolint: gosec
Copy link
Contributor

Choose a reason for hiding this comment

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

is this something we should worry about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the lint error arises because rootURL is a variable, but this is the assumption being made with the parameter of getServiceURL being rootURL

Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be exploited? Probably yes, but I guess validators who's using upnp know about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe it can be exploited, it is only used in one place in the code space and that is to get the serviceURL and urnDomain for NAT. I'm not in tune with possible exploits of this, but I may be wrong.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

🌟

@melekes melekes merged commit 866b343 into tendermint:develop Jun 21, 2019
unclezoro pushed a commit to unclezoro/tendermint that referenced this pull request Sep 6, 2019
- Govet issues fixed
- 1 gosec issue solved using nolint

Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants