[jira] [Updated] (SOLR-11687) SolrCore.getNewIndexDir falsely returns {dataDir}/index on any IOException reading index.properties
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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