For voting - if url does not contain http then add it (Quick Fix)#1531
Merged
jamescowens merged 2 commits intogridcoin-community:developmentfrom Oct 1, 2019
Merged
For voting - if url does not contain http then add it (Quick Fix)#1531jamescowens merged 2 commits intogridcoin-community:developmentfrom
jamescowens merged 2 commits intogridcoin-community:developmentfrom
Conversation
cyrossignol
reviewed
Sep 24, 2019
| item->url_ = QString::fromStdString(iterPoll.url); | ||
|
|
||
| if (iterPoll.url.find("http") == std::string::npos) | ||
| item->url_ = QString::fromStdString("https://" + iterPoll.url); |
Member
There was a problem hiding this comment.
I think we want to check that the URL starts with "http", not whether it exists anywhere within the string:
if (iterPoll.url.find("http") != 0) ...We could also left-trim whitespace from the string before checking and setting it to make the fallback more reliable, but it's not strictly necessary...
Member
Author
There was a problem hiding this comment.
i agree with that and idk why i did not do that.
Member
There was a problem hiding this comment.
@iFoggz Left trim looks good. Maybe we can write a function to support all whitespace later.
Do you want to do the check for "http" only at the beginning of the URL like I wrote above?
Member
There was a problem hiding this comment.
What about:
item->url_ = QString::fromStdString(iterPoll.url).trimmed();
if(!item->url_.startsWith("https://") && !item->url_.startsWith("http://"))
item->url_.prepend("https://");
Another alternative could be to switch to QUrl and use its setSchema method.
Member
There was a problem hiding this comment.
btw, are we sure that the server will support https? Maybe we should opt for http:// if no protocol is provided.
Member
Author
There was a problem hiding this comment.
alright will do this in a bit. and like ur suggestion. most servers forward http to http if they dont want to support http so http makes sense
Member
|
Looks like this is good to go. |
jamescowens
changed the title
jamescowens
changed the title
jamescowens
changed the title
jamescowens
added a commit
that referenced
this pull request
Oct 22, 2019
Added - Add testnet desktop launcher action for Linux #1516 (@caraka) - Shuffle vSideStakeAlloc if necessary to support sidestaking to more than 6 destinations #1532 (@jamescowens) - New Superblock format preparations for Fern #1526, #1542 (@jamescowens, @cyrossignol) - Multisigtools - Consolidate multisig unspent #1529 (@iFoggz) - Scanforunspent #1547 (@iFoggz) - consolidatemsunspent and scanforunspent bug fix #1561 (@iFoggz) - New banning misbehavior handling and Peers Tab on Debug Console #1537 (@jamescowens) - Reimplement getunconfirmedbalance rpc #1548 (@jamescowens) - Add CLI switch to display binary version #1553 (@cyrossignol) Changed - Select smallest coins for contracts #1519 (@iFoggz) - Move some functionality from miner to SelectCoinsForStaking + Respect the coin reserve setting + Randomize UTXO order #1525 (@iFoggz) - For voting - if url does not contain http then add it #1531 (@iFoggz) - Backport newer serialization facilities from Bitcoin #1535 (@cyrossignol) - Refactor ThreadSocketHandler2() Inactivity checks #1538 (@iFoggz) - Update outdated checkpoints #1539 (@barton2526) - Change needed to build Gridcoin for OSX using homebrew #1540 (@Git-Jiro) - Optimize scraper traffic for expiring manifests #1542 (@jamescowens) - Move legacy neural vote warnings to debug log level #1560 (@cyrossignol) - Change banlist save interval to 5 minutes #1564 (@jamescowens) - Change default rpcconsole.ui window size to better support new Peers tab #1566 (@jamescowens) Removed - Remove deprecated RSA weight and legacy kernel #1507 (@cyrossignol) Fixed - Clean up compiler warnings #1521 (@cyrossignol) - Handle missing external CPID in client_state.xml #1530 (@cyrossignol) - Support boost 1.70+ #1533 (@iFoggz) - Fix diagnostics failed to make connection to NTP server #1545 (@Git-Jiro) - Install manpages in correct system location #1546 (@Git-Jiro) - Fix ability to show help and version without a config file #1553 (@cyrossignol) - Refactor QT UI variable names to be more consistent, Fix Difficulty default #1563 (@barton2526) - Fix two regressions in previous UI refactor #1565 (@barton2526) - Fix "Owed" amount in output of "magnitude" RPC method #1569 (@cyrossignol)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #1513
Reasoning for placement of fix:
Rather then force a correct formatted http or https to url on contact I chose to do the fix in the voting dialog. I chose this because I don't think forcing a correct http,https,www in poll contract is overall beneficial. https:// is 8 characters and could take 4-5 bytes of transaction size as is. So lets just check if there is no http in the url and if there is not then correct the url to contain https instead in the voting dialog. This also allows for a http url to be set in a poll url in contract in case the said url server does not have https support. leaves this a little more compatible.
tACK with the poll in question on testnet