On Wed, Feb 16, 2011 at 17:08, Sergiu Dumitriu <[email protected]> wrote:
> On 02/16/2011 03:43 PM, Thomas Mortagne wrote:
>> On Wed, Feb 16, 2011 at 12:43, Sergiu Dumitriu<[email protected]> wrote:
>>> On 02/16/2011 11:50 AM, Thomas Mortagne wrote:
>>>> On Wed, Feb 16, 2011 at 11:36, Sergiu Dumitriu<[email protected]> wrote:
>>>>> On 02/16/2011 10:09 AM, tmortagne (SVN) wrote:
>>>>>> Author: tmortagne
>>>>>> Date: 2011-02-16 10:09:31 +0100 (Wed, 16 Feb 2011)
>>>>>> New Revision: 34718
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>> platform/core/branches/xwiki-core-2.7/xwiki-core/src/main/java/com/xpn/xwiki/store/XWikiHibernateBaseStore.java
>>>>>>
>>>>>> platform/core/branches/xwiki-core-2.7/xwiki-core/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java
>>>>>> Log:
>>>>>> XWIKI-5976: Cannot create subwiki named "lines"
>>>>>> Better (but a lot less elegant...) fix
>>>>>>
>>>>>> Modified:
>>>>>> platform/core/branches/xwiki-core-2.7/xwiki-core/src/main/java/com/xpn/xwiki/store/XWikiHibernateBaseStore.java
>>>>>> ===================================================================
>>>>>> ---
>>>>>> platform/core/branches/xwiki-core-2.7/xwiki-core/src/main/java/com/xpn/xwiki/store/XWikiHibernateBaseStore.java
>>>>>> 2011-02-16 09:09:21 UTC (rev 34717)
>>>>>> +++
>>>>>> platform/core/branches/xwiki-core-2.7/xwiki-core/src/main/java/com/xpn/xwiki/store/XWikiHibernateBaseStore.java
>>>>>> 2011-02-16 09:09:31 UTC (rev 34718)
>>>>>> @@ -504,7 +504,6 @@
>>>>>> }
>>>>>> } catch (Exception e) {
>>>>>> }
>>>>>> - ;
>>>>>> try {
>>>>>> if (bTransaction) {
>>>>>> endTransaction(context, true);
>>>>>> @@ -600,13 +599,14 @@
>>>>>>
>>>>>> if (context.getDatabase() != null) {
>>>>>> String schemaName =
>>>>>> getSchemaFromWikiName(context);
>>>>>> + String escapedSchemaName = escapeSchema(schemaName,
>>>>>> context);
>>>>>>
>>>>>> DatabaseProduct databaseProduct =
>>>>>> getDatabaseProductName(context);
>>>>>> if (DatabaseProduct.ORACLE == databaseProduct) {
>>>>>> Statement stmt = null;
>>>>>> try {
>>>>>> stmt =
>>>>>> session.connection().createStatement();
>>>>>> - stmt.execute("alter session set
>>>>>> current_schema = " + schemaName);
>>>>>> + stmt.execute("alter session set
>>>>>> current_schema = " + escapedSchemaName);
>>>>>> } finally {
>>>>>> try {
>>>>>> if (stmt != null) {
>>>>>> @@ -620,7 +620,7 @@
>>>>>> Statement stmt = null;
>>>>>> try {
>>>>>> stmt =
>>>>>> session.connection().createStatement();
>>>>>> - stmt.execute("SET SCHEMA " + schemaName);
>>>>>> + stmt.execute("SET SCHEMA " +
>>>>>> escapedSchemaName);
>>>>>> } finally {
>>>>>> try {
>>>>>> if (stmt != null) {
>>>>>> @@ -648,6 +648,29 @@
>>>>>> }
>>>>>>
>>>>>> /**
>>>>>> + * Escape schema name depending of the database engine.
>>>>>> + *
>>>>>> + * @param schema the schema name to escape
>>>>>> + * @param context the XWiki context to get database engine
>>>>>> identifier
>>>>>> + * @return the escaped version
>>>>>> + */
>>>>>> + protected String escapeSchema(String schema, XWikiContext context)
>>>>>> + {
>>>>>> + DatabaseProduct databaseProduct =
>>>>>> getDatabaseProductName(context);
>>>>>> +
>>>>>> + String escapedSchema;
>>>>>
>>>>> You should use this instead:
>>>>>
>>>>> escapedSchema = dialect.openQuote() + schema + dialect.closeQuote();
>>>>
>>>> Ok thanks
>>>>
>>>>>
>>>>> I think nobody wants to use ` or " in the wiki name, so there shouldn't
>>>>> be a need for doubling them.
>>>>
>>>> No sure about that. We have to do something, either remove or properly
>>>> escape then otherwise it's not very safe
>>>
>>> OK, you can double the openQuote() character. For SQLServer dialect you
>>> have to do it differently, though:
>>>
>>> replace("[", "[[]")
>>>
>>> Although, simple doubling isn't enough, for example trying to create
>>> this database will drop the xyz database:
>>>
>>> x\`; drop database xyz; \`
>>>
>>> turns into:
>>>
>>> create database `x\``; drop database xxx; \```;
>>>
>>> which fails to create the first database (invalid name), drops the
>>> second database, and fails to execute the ` command. I tested it on the
>>> mysql console, not through Hibernate.
>>
>> That's weird since
>> http://dev.mysql.com/doc/refman/5.0/en/identifiers.html says
>> "Identifier quote characters can be included within an identifier if
>> you quote the identifier. If the character to be included within the
>> identifier is the same as that used to quote the identifier itself,
>> then you need to double the character."
>
> Yes, that is correct, but \ is an escape character.
Sorry i did not seen the \ in your example.
>
> So, normally doubling ` will have the desired effect:
>
> create database that`s life; -> invalid, error.
> create database `that``s life`; -> valid, OK.
> create database `that\``s life`; -> invalid, since \` actually escapes
> the first backquote, breaking the special meaning of ``. That's not a
> double backquote which translates into a single literal backquote, but a
> literal backquote followed by an unmatched, unescape backquote which
> closes the starting backquote early.
>
> This holds true for " as well, and this is why escapetool.sql is not
> enough to prevent SQL injections and parameterized queries should be
> used everywhere.
Then the best is to use \ to escape, I don't understand why they don't
talk about that in the documentation if that a way to escape it.
>
>>>
>>>>>
>>>>> BTW, SQLServer uses [ ] for quoting.
>>>>
>>>> You mean DB2 ?
>>>
>>> I mean Microsoft's SQL Server, their complete lack of imagination makes
>>> it hard to distinguish their product.
>>
>> I understood that but you I don't see any support for MS SQL Server in
>> DatabaseProduct and i don't see why I would add it now just for that.
>
> Well, there are several users that run XWiki on top of MSSQL, and even
> though it's not highly maintained by the developers, we should at least
> try not to break things on purpose.
I don't see what is broken. Having ] was not working anyway since
nothing was escaped. It's not about breaking on purpose it's about
having a special handling for this database here and I'm not sure it's
critical here.
>
>>>
>>>>>
>>>>>> + if (DatabaseProduct.MYSQL == databaseProduct) {
>>>>>> + // MySQL does not use SQL92 escaping syntax by default
>>>>>> + escapedSchema = "`" + schema.replace("`", "``") + "`";
>>>>>> + } else {
>>>>>> + // Use SQL92 escape syntax
>>>>>> + escapedSchema = "\"" + schema.replace("\"", "\"\"") + "\"";
>>>>>> + }
>>>>>> +
>>>>>> + return escapedSchema;
>>>>>> + }
>>>>>> +
>>>>>> + /**
>>>>>> * Begins a transaction
>>>>>> *
>>>>>> * @param context
>
>
> --
> Sergiu Dumitriu
> http://purl.org/net/sergiu/
> _______________________________________________
> devs mailing list
> [email protected]
> http://lists.xwiki.org/mailman/listinfo/devs
>
--
Thomas Mortagne
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
