[ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16271690#comment-16271690
 ] 

Hoss Man commented on SOLR-11687:
---------------------------------

erick: i haven't audited all of the code paths that call this method (did you?) 
but the one thing that jumps out at me in your patch is your choice to throw an 
explicit {{new RuntimeException(e)}} ... why not {{new 
SolrException(SERVER_ERROR, "Some message about index.properties", e)}} (since 
SolrException is also a RuntimeException) 

?



> SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException 
> reading index.properties
> ---------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-11687
>                 URL: https://issues.apache.org/jira/browse/SOLR-11687
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Erick Erickson
>            Assignee: Erick Erickson
>         Attachments: SOLR-11687.patch
>
>
> I'll link the originating Solr JIRA in a minute (many thanks Nikolay). 
> right at the top of this method we have this:
> {code}
> String result = dataDir + "index/";
> {code}
> If, for any reason, the method doesn't complete properly, the "result" is 
> still returned. Now for instance, down in SolrCore.cleanupOldIndexDirectories 
> the "old" directory is dataDir/index which may point to the current index.
> This seems particularly dangerous:
> {code}
>        try {
>           p.load(new InputStreamReader(is, StandardCharsets.UTF_8));
>           String s = p.getProperty("index");
>           if (s != null && s.trim().length() > 0) {
>               result = dataDir + s;
>           }
>         } catch (Exception e) {
>           log.error("Unable to load " + IndexFetcher.INDEX_PROPERTIES, e);
>         } finally {
>           IOUtils.closeQuietly(is);
>         }
> {code}
> Should "p.load" fail for any reason whatsoever, we'll still return 
> dataDir/index.
> Anyone want to chime on on what the expectations are here before I dive in?



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to