Opened 13 years ago

Closed 12 years ago

#3072 closed defect (fixed)

MythGame: Apostrophe or single quote in filename breaks selection.

Reported by: spikeygg@… Owned by: greg
Priority: minor Milestone: 0.21
Component: mythgame Version: 0.20
Severity: medium Keywords:
Cc: Ticket locked: no

Description

I have many roms with a single quote in the filename. These roms cannot be selected because their filenames contain one or more single quotes.

Attachments (1)

MySqlInfo.txt (2.5 KB) - added by spikeygg@… 13 years ago.
The requested mysql entries.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 13 years ago by greg

Resolution: worksforme
Status: newclosed

I've tried to reproduce this on both 0.20-fixes and SVN trunk and so far it works fine for me everytime.

If this is still a problem for you, reopen with more details including the full entry from your database of the gamemetadata for one of the affect filenames and from the gameplayers table for that player

Changed 13 years ago by spikeygg@…

Attachment: MySqlInfo.txt added

The requested mysql entries.

comment:2 Changed 13 years ago by spikeygg@…

Resolution: worksforme
Status: closedreopened

The problem persists. I'm currently running SVN 13352. I have attached the entries for my game player as well as a rom that does not work from the menu. (sorry for the 3 week delay:)

-Greg

comment:3 in reply to:  2 Changed 12 years ago by greg

The problem persists. I'm currently running SVN 13352. I have attached the entries for my game player as well as a rom that does not work from the menu. (sorry for the 3 week delay:)

I'm curious, What version of MySQL are you using? As well, when you highlight the game in the list, and then hit Enter to try and play it, what is output by the frontend? Normally there will be a line detailing the commandline being generated to try and launch the game.

Also.... what shell are you using? Bash?

comment:4 Changed 12 years ago by spikeygg@…

I'm using: mysql Ver 14.12 Distrib 5.0.38, for pc-linux-gnu (x86_64) using readline 5.2

IIRC, nothing is reported when I attempt to launch a game with an apostrophe. I'll have to try it again tonight when I get home from work. I'll respond with the messages if there are any.

I am using /bin/bash as my shell.

comment:5 Changed 12 years ago by spikeygg@…

So, I've found that while rolling through the game selections for the first time if I move the selector to a game with an apostrophe the frontend will spit out:

QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record
QSqlQuery::value: not positioned on a valid record

Subsequent selections of the "broken names" causes no output.

I doubt this is a problem but every time I move the selector any other game it complains with this:

MythThemedDialog.o: something is requesting a screen update of zero size. A widget probably has not done a calculateScreeArea(). Will redraw the whole screen (inefficient!).

The frontend says nothing if I attempt to run/execute the games which contain an apostrophe.

-Greg

comment:6 Changed 12 years ago by mike@…

Just wanted to add my similar problem to this. I'm running atrpms-bleeding (SVN version 13756) and roms with apostrophes in the filename do not show up at all in the game selection menu. For example rom with filename - Yoshi's Story - will not be detected by the game scan.

I'm not using the romsdb or in depth scan. I have tried checking the 'use filename' option and re-scanning the collection, this did not make the missing roms appear.

I haven't tried watching any logs while doing the actual scan. I can try that if you think it will provide any useful info.

Cheers, Mike

comment:7 Changed 12 years ago by mythtv@…

I noticed the same problem recently in my MythTV installation (I even updated to the latest SVN as of Nov 3, 2007). I never noticed it was due to the apostrophes until I saw this ticket. After weeding through the code, it appears this was broken at revision [12542]. In that revision, the double quotes in the queries were changed to single quotes for compatibility reasons (which is correct), but since the strings are now enclosed in single quotes, you will get SQL errors for files that have apostrophes in them.

The fix is to escape the string before it is injected into the SQL statement. In SQL, a single quote is escaped with another single quote. For example, in rominfo.cpp, this change (one of many that are needed) will fix the display of the game info in the ROM browser.

......

QString thequery = "SELECT system,gamename,genre,year,romname,favorite,"

"rompath,country,crc_value,diskcount,gametype,publisher," "version FROM gamemetadata WHERE gamename='" + gamename.replace("'", "") + "'";

......

Obviously the cleanest way is to have some sort of escaping function (possibly in the MSqlQuery class) or to use prepared SQL statements. In the change above, it also alters the original string (which may not be a good idea depending on the circumstances). I'm not quite familiar enough with the MythTV code to provide a patch myself, but if no one else is willing to give it a shot, I may be able to get around to it sometime soon.

comment:8 Changed 12 years ago by anonymous

Well, I suppose for 'clueless Windows uses' this might be a good fix - but not really relevant for intelligent people that recognize that apostrophes and quotes, spaces, slashes, and other cli-significant characters dont belong in filenames to begin with., and if for some reason they encounter such a file, automatically rename away the offending characters.

Personally, I'd be happy if all unix-based filesystems automatically converted such characters to single dots anytime they were used.

Sane characters to use in filenames: Alphabetic a thru z (Upper and lowercase), numberic digits, dash, underscore, colon, plus, tilde and dot.

Characters only idiots or morons use: spaces, slashes, ticks, quotes, asterisk, dollar-sign, commas, pipes, greater/less than, ampersand, etc.

comment:9 Changed 12 years ago by mythtv@…

I think it's obvious from your snide comments that you are nothing more than a troll trying to stir up trouble. It also shows you really don't understand the problem at hand.

Back on topic, the sample code change that was posted to illustrate a fix (but not necessarily the most ideal fix) is dealing with a field in the database that is completely separate from the filename. The romname field of gamemetadata holds the filename, while the gamename field holds the name of the game to display in the selection list. They can be changed independently of each other. Does this mean you are saying we should stop using all of the "idiot or moron" characters when giving a friendly display name for our data too?

There are other SQL statements that do pull information based upon on the romname (which does equate to the filename), but that doesn't mean they should not be fixed. This is no different than someone implementing SQL injection protection on a web site, in the event that the programmer cannot make any guaranteed judgments about the safety of the input.

comment:10 Changed 12 years ago by greg

(In [15009])

references #3072 and updates all but 1 query to use bindvalues to avoid issues with values containing quotes/ticks. There is still one more to be converted. Once that is done anyone having problems with quotes in filenames shouldn't

be having anymore trouble.

Also replaces all cerr/cout left in the code with VERBOSE() entries to make sure the info gets into the normal logs

and fixes one bug where a rom found in romDB might have a blank Genre.

comment:11 Changed 12 years ago by greg

Milestone: unknown0.21

comment:12 Changed 12 years ago by greg

Resolution: fixed
Status: newclosed

(In [15665])

Closes #3072 by changing the last bit of code to use bind values.

Also adds a new sort by Publisher option.

Note: See TracTickets for help on using tickets.