Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#7512 closed defect (duplicate)

Problem building path for dojo.query when an attribute's value contains a "=" (equals sign).

Reported by: otasyn Owned by: dante
Priority: high Milestone: 1.2
Component: Query Version: 1.1.1
Keywords: dojo.query getQueryParts Cc:
Blocked By: Blocking:

Description (last modified by dante)

I've never filed one of these before, so forgive me if I do something wrong.

When running dojo.query(), if the queryStr has an attribute value that contains a "=" (equals sign), then dojo throws an exception (failure in exprssion...). For example:

dojo.query("option[value='==']");

changes the queryStr to ".//option[@value='==''']" . This is incorrect because dojo thought that the last "=" it saw in the brackets determined the end of the attribute name "value='=" instead of just "value" .

The problem lies in the function getQueryParts(). This function contains a section where it loops through every character of the original queryStr. In this function, the queryStr is defined as "query". I am using dojo.js.uncompressed.js from Dojo 1.1.1. Around Line 5678.

for(; lc=cc, cc=query.charAt(x),x<ql; x++){
    if(lc == "\\"){ continue; }
    if(!currentPart){
        // NOTE: I hate all this alloc, but it's shorter than writing tons of if's
        pStart = x;
        currentPart = {
            query: null,
            pseudos: [],
            attrs: [],
            classes: [],
            tag: null,
            oper: null,
            id: null
        };
        inTag = x;
    }

    if(inBrackets >= 0){
        // look for a the close first
        if(cc == "]"){
            if(!_cp.attr){
                _cp.attr = ts(inBrackets+1, x);
            }else{
                _cp.matchFor = ts((inMatchFor||inBrackets+1), x);
            }
            var cmf = _cp.matchFor;
            if(cmf){
                if(    (cmf.charAt(0) == '"') || (cmf.charAt(0)  == "'") ){
                    _cp.matchFor = cmf.substring(1, cmf.length-1);
                }
            }
            currentPart.attrs.push(_cp);
            _cp = null; // necessaray?
            inBrackets = inMatchFor = -1;
        }else if(cc == "="){
            var addToCc = ("|~^$*".indexOf(lc) >=0 ) ? lc : "";
            _cp.type = addToCc+cc;
            _cp.attr = ts(inBrackets+1, x-addToCc.length);
            inMatchFor = x+1;
        }
        // now look for other clause parts
    }else if(inParens >= 0){
        if(cc == ")"){
            if(inPseudo >= 0){
                _cp.value = ts(inParens+1, x);
            }
            inPseudo = inParens = -1;
        }
    }else if(cc == "#"){
        endAll();
        inId = x+1;
    }else if(cc == "."){
        endAll();
        inClass = x;
    }else if(cc == ":"){
        endAll();
        inPseudo = x;
    }else if(cc == "["){
        endAll();
        inBrackets = x;
        _cp = {
            /*=====
            attr: null, type: null, matchFor: null
            =====*/
        };
    }else if(cc == "("){
        if(inPseudo >= 0){
            _cp = { 
                name: ts(inPseudo+1, x), 
                value: null
            }
            currentPart.pseudos.push(_cp);
        }
        inParens = x;
    }else if(cc == " " && lc != cc){
        // note that we expect the string to be " " terminated
        endAll();
        if(inPseudo >= 0){
            currentPart.pseudos.push({ name: ts(inPseudo+1, x) });
        }
        currentPart.hasLoops = (    
                currentPart.pseudos.length || 
                currentPart.attrs.length || 
                currentPart.classes.length    );
        currentPart.query = ts(pStart, x);
        currentPart.tag = (currentPart["oper"]) ? null : (currentPart.tag || "*");
        qparts.push(currentPart);
        currentPart = null;
    }
}

In the section, when it finds brackets, it works inside this section of code. Around Line 5751.

if(inBrackets >= 0){
    // look for a the close first
    if(cc == "]"){
        if(!_cp.attr){
            _cp.attr = ts(inBrackets+1, x);
        }else{
            _cp.matchFor = ts((inMatchFor||inBrackets+1), x);
        }
        var cmf = _cp.matchFor;
        if(cmf){
            if(    (cmf.charAt(0) == '"') || (cmf.charAt(0)  == "'") ){
                _cp.matchFor = cmf.substring(1, cmf.length-1);
            }
        }
        currentPart.attrs.push(_cp);


        _cp = null; // necessaray?
        inBrackets = inMatchFor = -1;
    }else if(cc == "="){
        var addToCc = ("|~^$*".indexOf(lc) >=0 ) ? lc : "";
        _cp.type = addToCc+cc;
        _cp.attr = ts(inBrackets+1, x-addToCc.length);
        inMatchFor = x+1;
    }
    // now look for other clause parts
}

So, while still working inside brackets, it looks for either "]" in order to finish up or "=" in order to determine an attribute name. The problem is that it should only look for the first "=", then quit looking. Everything after the "=" should be related to the value.

So, I propose this solution. Somewhere at the beginning of the function, declare a variable. For example:

var attrNameFound = false;

At the end of the if(cc == "]") block, add:

attrNameFound = false;

Inside the if(cc == "=") block, wrap the contents in another if to check if the attribute name has already been determined:

if (!attrNameFound) {
    var addToCc = ("|~^$*".indexOf(lc) >=0 ) ? lc : "";
    _cp.type = addToCc+cc;
    _cp.attr = ts(inBrackets+1, x-addToCc.length);
    inMatchFor = x+1;
    attrNameFound = true;
}

That solved it for me, but my solution is only a suggestion. Maybe someone knows a better way.

Change History (3)

comment:1 Changed 11 years ago by dante

Description: modified (diff)
Milestone: tbd1.2
Owner: changed from anonymous to dante

this seems like a dup of #7479 ... can you try that patch against your tests and report back please? neither tickets have a unit test to use for this failure, so to hear back would be a start.

comment:2 Changed 11 years ago by dante

Description: modified (diff)
Resolution: duplicate
Status: newclosed

comment:3 Changed 10 years ago by bill

Component: CoreQuery
Note: See TracTickets for help on using tickets.