Solr
  1. Solr
  2. SOLR-3040

Avoid use of qt param for the DIH in the admin UI

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: web gui
    • Labels:
      None

      Description

      I really, really dislike that the qt parameter can used to refer to request handlers starting with a '/', especially for non-search handlers. The admin UI has one place I am aware of that attempts to do this, which is the DIH's admin page. Since we have two UIs in trunk, the new and old, there are actually two UIs where this occurs, and the old UI has two related files that need updating, in order to address this issue.

      An example URL generated by the UI today is this:
      http://localhost:8983/solr/rss/select?qt=/dataimport&command=show-config
      And here it is without using qt:
      http://localhost:8983/solr/rss/dataimport?command=show-config

      I do realize that fixing this issue as I do in my patch will make the UI not work if the DIH isn't registered with a leading '/', but honestly I wonder who if anyone out there does that. And even then, it's easily rectified.

        Issue Links

          Activity

          Hide
          David Smiley added a comment -

          I'm going to commit this within 24 hours unless someone says otherwise.

          Show
          David Smiley added a comment - I'm going to commit this within 24 hours unless someone says otherwise.
          Hide
          Erik Hatcher added a comment -

          +0 - just be sure to call out a sufficient CHANGES entry that mentions that non-/-prefixed DIH handlers will need to be changed. And maybe only do this change on 4.x rather than 3.x since it is kind of an unnecessary break.

          Don't get me wrong, I'm all for getting Solr to handle path-based request handlers without qt - I'd rather all request handlers be path-based personally (even if I'm the one that committed qt=/rh to work, as this was for consistency on the client-side) and to get rid of qt fully (but that's a separate issue).

          Show
          Erik Hatcher added a comment - +0 - just be sure to call out a sufficient CHANGES entry that mentions that non-/-prefixed DIH handlers will need to be changed. And maybe only do this change on 4.x rather than 3.x since it is kind of an unnecessary break. Don't get me wrong, I'm all for getting Solr to handle path-based request handlers without qt - I'd rather all request handlers be path-based personally (even if I'm the one that committed qt=/rh to work, as this was for consistency on the client-side) and to get rid of qt fully (but that's a separate issue).
          Hide
          David Smiley added a comment -

          Thanks for reminding me to add a CHANGES entry because I definitely should. I plan on doing this on both branches because I think it would be both unlikely and a bad idea for someone to not have a leading slash on their DIH handler, and it's trivial for them to fix, and if they insist on not fixing it then this piece of the admin UI won't work which is hardly the end of the world – it's just an html page.

          The rest of your comment pertains to SOLR-1233 which as you surmised is what I'll be doing next. Lets discuss that issue there, not here.

          Show
          David Smiley added a comment - Thanks for reminding me to add a CHANGES entry because I definitely should. I plan on doing this on both branches because I think it would be both unlikely and a bad idea for someone to not have a leading slash on their DIH handler, and it's trivial for them to fix, and if they insist on not fixing it then this piece of the admin UI won't work which is hardly the end of the world – it's just an html page. The rest of your comment pertains to SOLR-1233 which as you surmised is what I'll be doing next. Lets discuss that issue there, not here.
          Hide
          Erik Hatcher added a comment -

          Sorry, I was thinking this was more externally visible of a change, in that the URLs you were changing were ones external systems would hit, but now I'm clear.... updated to +1 based on clarity It's all internal to the DIH control panel (which arguably shouldn't be embedded in the core webapp anyway, eh).

          Show
          Erik Hatcher added a comment - Sorry, I was thinking this was more externally visible of a change, in that the URLs you were changing were ones external systems would hit, but now I'm clear.... updated to +1 based on clarity It's all internal to the DIH control panel (which arguably shouldn't be embedded in the core webapp anyway, eh).
          Hide
          David Smiley added a comment -

          Committed branch_3x: r1293105 trunk: r1293102

          CHANGES.txt entry underneath upgrade instructions:

          * SOLR-3040: The DIH's admin UI (dataimport.jsp) now requires
           DIH request handlers to start with a '/'. (dsmiley)
          

          FYI it turned out that in the span of time since when I did the first patch and when I committed now, the new admin UI already gained this limitation (leading '/' required). Wether deliberate or accidental, I'm glad.

          Show
          David Smiley added a comment - Committed branch_3x: r1293105 trunk: r1293102 CHANGES.txt entry underneath upgrade instructions: * SOLR-3040: The DIH's admin UI (dataimport.jsp) now requires DIH request handlers to start with a '/'. (dsmiley) FYI it turned out that in the span of time since when I did the first patch and when I committed now, the new admin UI already gained this limitation (leading '/' required). Wether deliberate or accidental, I'm glad.
          Hide
          Chris Male added a comment -

          Thanks for tackling this David, its a good improvement.

          Show
          Chris Male added a comment - Thanks for tackling this David, its a good improvement.

            People

            • Assignee:
              David Smiley
              Reporter:
              David Smiley
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development