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.

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

[GUI] Fix proposal name and URL size limit validation #2672

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

furszy
Copy link

@furszy furszy commented Dec 9, 2021

Fixes #2666 crash. Which is related to the size, in bytes, of the proposal name and/or the URL that can exceed the maximum allowed size, then crash during the limited string unserialization process.

@furszy furszy self-assigned this Dec 9, 2021
@furszy furszy added this to the 5.4.0 milestone Dec 9, 2021
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

looking good, just missing a string translation.

also, if we want the proposal name field to have the same real-time visual response (red border) in regards to over-length (can happen when using unicode characters), can use this diff:

Index: src/qt/pivx/createproposaldialog.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/qt/pivx/createproposaldialog.cpp b/src/qt/pivx/createproposaldialog.cpp
--- a/src/qt/pivx/createproposaldialog.cpp	(revision 7342206ca7700d2329b3f6dd06e2cf17b69288f1)
+++ b/src/qt/pivx/createproposaldialog.cpp	(date 1639089820920)
@@ -137,7 +137,7 @@
 
 void CreateProposalDialog::propNameChanged(const QString& newText)
 {
-    bool isValid = !newText.isEmpty() && IsValidUTF8(newText.toStdString());
+    bool isValid = !newText.isEmpty() && IsValidUTF8(newText.toStdString()) && govModel->validatePropName(newText).getRes();
     setCssEditLine(ui->lineEditPropName, isValid, true);
 }
 

src/qt/pivx/governancemodel.cpp Outdated Show resolved Hide resolved
@furszy furszy force-pushed the 2021_gui_fix_create_prop_crash branch from ed4112c to 9d86871 Compare December 9, 2021 23:49
@furszy
Copy link
Author

furszy commented Dec 9, 2021

done, quick update. I missed to add it to the text change signal validation too.

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 9d86871

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK 9d86871

@furszy furszy merged commit 7caa108 into PIVX-Project:master Dec 10, 2021
furszy referenced this pull request in furszy/bitcoin-core Dec 11, 2021
furszy referenced this pull request in furszy/bitcoin-core Dec 11, 2021
furszy added a commit that referenced this pull request Dec 15, 2021
c9b1909 [net] Remove assert(nMaxInbound > 0) (MarcoFalke)
d629df0 [Refactor] Add ReloadMapSeen function to reload "seen" votes (random-zebra)
7427d44 test: add coverage for the incremental budget sync flow. (furszy)
241627c Budget incremental sync: do not clear seen maps when relay status is reset. (furszy)
61477a5 [Refactor] Budget: check mn-enabled/vote-time before adding to mapSeen (random-zebra)
318fc7c Budget: limit orphan votes maps to 10k entries max. (furszy)
55f3ef9 [Refactor] Simplify CBudgetManager::CheckOrphanVotes() Github-Pull: #2659 Rebased-From: d04f5be (random-zebra)
e5ad197 Budget: Decouple the all-in-one `Budget::Sync()` into two functions: (1) single items sync requests and (2) full budget sync requests. (furszy)
5fd74e4 tiertwo sync: fix orphans votes maps linking single prop/bud hash to a single vote, discarding the previous stored item every time that a new orphan arrives. Blocking any follow-up reception of the orphan votes that were discarded, prohibiting its connection forever. (furszy)
4cdb587 test: add coverage for single proposal sync based on an orphan proposal vote reception and single finalization sync based on an orphan finalization vote. (furszy)
979bb2c budget: generalize budget full sync items relay. (furszy)
736363d Budget sync: Remove useless extra item sync p2p msgs roundtrip (no need to broadcast an inv if the peer is asking for the item directly..) and stop walking through the two maps (budgets and proposals), locking their mutexes, when the peer is requesting a single proposal/budfin. (furszy)
fe6fe56 Tiertwo: do not block peers if they requested a single proposal or single budget finalization. (furszy)
ebbef96 masternodesync: connect the not-connected ProcessSyncStatusMsg. (furszy)
281c061 masternodesync: fix extra `RequestedMNAttempt` increment in the sporks flow. (furszy)
d95fb9b masternodesync: decrease mnw sync threshold to 4 peers. (furszy)
cfc15b6 masternodesync: reset last budget item time before start syncing it. (furszy)
94f1c52 masternodesync: re-format peer version check. (furszy)
946444e Fix ignoring tx data requests when fPauseSend is set on a peer (Matt Corallo)
10cedd0 GUI governance: add missing strings to the translations process. (furszy)
dc10e17 GUI governance: fix proposal name and URL size limit validation. (furszy)
594acc6 [GUI] Fix governance nav button hover css (Fuzzbawls)
fd8d844 GUI: governance, IBD check before opening the proposal creation wizard. (furszy)
33f5dd8 [GUI] Fix proposal tooltip menu location skewing (Fuzzbawls)
55619cc GUI: Differentiate Budget payment transaction records from MNs block reward records. (furszy)
139596e Refactor: Move MN block reward to chainparams. (furszy)
1a55752 GUI: Fix remaining item naming conflicts (Fuzzbawls)
5925150 GUI: Use sorted layout/spacer names in governance UIs (Fuzzbawls)
dfeb3c7 [GUI] Don't show UTXO locking options for shield notes (Fuzzbawls)
6dd96b9 [BUG] GUI: invalid locking of shield notes in coin control Github-Pull: #2661 Rebased-From: d281cde (random-zebra)
1d7f86e Refactor: remove circ depend. primitives/transaction <-> script/standard (random-zebra)
cb3a5b4 Refactor: remove circ dependency checkpoints <-> chainparams (random-zebra)
e02e70e Refactor: remove circ dependency checkpoints <-> validation (random-zebra)
e1a5d0a Break circular dependency: init -> * -> init by extracting shutdown.h (random-zebra)
2d29c42 Cleanup: remove unused init.h includes Github-Pull: #2646 Rebased-From: b09422f (random-zebra)
95c7f2e cleanup: fix header includes sorting and copy Github-Pull: #2642 Rebased-From: 525c2e7 (random-zebra)
fdd4045 scripted-diff: replace boost::optional with Optional<> wrapper (random-zebra)

Pull request description:

  List of PRs included:

  * #2642
  * #2646
  * #2643
  * #2644
  * #2661
  * #2662
  * #2663
  * #2671
  * #2664
  * #2670
  * #2665
  * #2672
  * #2659
  * #2677
  * #2682

ACKs for top commit:
  random-zebra:
    ACK c9b1909
  Fuzzbawls:
    ACK c9b1909

Tree-SHA512: 34871cd7d527302ec6bd3924f4ca4322116952c8b69317831b2652165fbf1c566f893dace9a2998823c6781c4e578c2eeb9bc367fa39f632235aee7f45a68306
@furszy furszy deleted the 2021_gui_fix_create_prop_crash branch November 29, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Wallet crashes if Amount entered in the field starts with 0
3 participants