[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-12-07 Thread Adrien Grand (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Adrien Grand updated SOLR-11687:

Fix Version/s: master (8.0)

> 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
> Fix For: 7.2, master (8.0)
>
> Attachments: SOLR-11687.patch, SOLR-11687.patch, SOLR-11687.patch, 
> SOLR-11687_alt.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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-12-04 Thread Varun Thacker (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Varun Thacker updated SOLR-11687:
-
Attachment: SOLR-11687_alt.patch

Hi Erick

Is this change enough then? 

> 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, SOLR-11687.patch, SOLR-11687.patch, 
> SOLR-11687_alt.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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-12-04 Thread Erick Erickson (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Erick Erickson updated SOLR-11687:
--
Attachment: SOLR-11687.patch

Final patch with CHANGES.txt

> 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, SOLR-11687.patch, 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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-12-03 Thread Erick Erickson (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Erick Erickson updated SOLR-11687:
--
Attachment: SOLR-11687.patch

Final patch (except for CHANGES.txt). I'll commit tomorrow (Monday) unless 
there are objections after running another round of precommit/tests.

There's a bit of extra noise. When I adopted Hoss' suggestion to use 
SolrException rather than RuntimeException, IntelliJ added an import and 
changed all the 
SolrException.ErrorCode.SERVER_ERROR
to
ErrorCode.SERVER_ERROR

but I'm going to leave that part in and easily ignorable.

> 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, 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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-11-28 Thread Erick Erickson (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Erick Erickson updated SOLR-11687:
--
Attachment: SOLR-11687.patch

Here's a patch for comment. All tests pass. 

Mostly it reorganizes the decision whether to return a string that's the 
indexDir or throw an exception.

There are  a couple of nocommits in here, mostly to draw people's attention to 
some assumptions if they have any opinion.

Comments welcome. If there aren't any objections I'll commit this in a few days 
after doing some additional testing.

> 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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties

2017-11-27 Thread Hoss Man (JIRA)

 [ 
https://issues.apache.org/jira/browse/SOLR-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Hoss Man updated SOLR-11687:

Description: 
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?

  was:
I'll link the originating Solr JIRA in a minute (many thanks Nikolay). 

right at the top of this method we have this:

String result = dataDir + "index/";

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:
{{
   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);
}
 }}

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?

Summary: SolrCore.getNewIndexDir falsely returns {dataDir}/index on any 
IOException reading index.properties  (was: SolrCore.getNewIndexDir returns the 
current index on most exceptions)

(issue edited to reformat the description and clarify the summary)

> 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
>
> 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: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org