Code review comment for lp:~nick-dedekind/unity8/side-stage-redesign

Revision history for this message
Michael Terry (mterry) wrote :

I started looking at this, but realized that my only tablet suitable for testing this is a manta, and it's busted these days. :( So maybe someone else should actually do the review. But just one quick comment:

+ const QString queryString = QStringLiteral("INSERT OR REPLACE INTO stage (appId, stage) values ('%1', '%2');")
+ .arg(appId)
+ .arg((int)stage);

I know that appIds are not the wild west of string input, but it makes me very nervous to see an unescaped sprintf fed into SQL. Is there an easy QString call or similar that we can make to give us 100% future proof peace of mind?

I mean, I assume the appId rules are sane -- only alphanumeric and hyphen maybe? But is u8 actually enforcing that, or are we just trusting that other components have? Wouldn't hurt to either add a comment here explaining exactly why we trust this input or simply escape it ourselves.

Same for getStage right below this.

« Back to merge proposal