Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9623

Disable remote streaming by default

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 7.0
    • Component/s: security
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:

      Description

      As we set more and more config settings suitable for production use, perhaps it is time to disable remoteStreaming by default, and document how to enable it.

      1. SOLR-9623.patch
        47 kB
        Jan Høydahl
      2. SOLR-9623.patch
        47 kB
        Jan Høydahl
      3. SOLR-9623.patch
        47 kB
        Jan Høydahl
      4. SOLR-9623.patch
        36 kB
        Jan Høydahl

        Issue Links

          Activity

          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit a4f27bbfbb1d0542a95e13507555cf74ebccbe45 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a4f27bb ]

          SOLR-9623: Fix test errors related to some test expecting streaming to be enabled

          Show
          jira-bot ASF subversion and git services added a comment - Commit a4f27bbfbb1d0542a95e13507555cf74ebccbe45 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a4f27bb ] SOLR-9623 : Fix test errors related to some test expecting streaming to be enabled
          Hide
          janhoy Jan Høydahl added a comment -

          Thanks Steve, I'm on the case

          Show
          janhoy Jan Høydahl added a comment - Thanks Steve, I'm on the case
          Hide
          steve_rowe Steve Rowe added a comment -

          Also these SolrRequestParserTest failures ("Remote Streaming is disabled.") https://jenkins.thetaphi.de/job/Lucene-Solr-master-Solaris/1384/:

             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=SolrRequestParserTest -Dtests.method=testStreamURL -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=de-DE -Dtests.timezone=America/Aruba -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
             [junit4] ERROR   0.01s J2 | SolrRequestParserTest.testStreamURL <<<
             [junit4]    > Throwable #1: org.apache.solr.common.SolrException: Remote Streaming is disabled.
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:BFA5EC71A3C03B8B]:0)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:190)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:178)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParserTest.testStreamURL(SolrRequestParserTest.java:140)
          [...]
             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=SolrRequestParserTest -Dtests.method=testStreamFile -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=de-DE -Dtests.timezone=America/Aruba -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
             [junit4] ERROR   0.00s J2 | SolrRequestParserTest.testStreamFile <<<
             [junit4]    > Throwable #1: org.apache.solr.common.SolrException: Remote Streaming is disabled.
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:795923A829FF9159]:0)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:205)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:178)
             [junit4]    > 	at org.apache.solr.servlet.SolrRequestParserTest.testStreamFile(SolrRequestParserTest.java:162)
          
          Show
          steve_rowe Steve Rowe added a comment - Also these SolrRequestParserTest failures ("Remote Streaming is disabled.") https://jenkins.thetaphi.de/job/Lucene-Solr-master-Solaris/1384/ : [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=SolrRequestParserTest -Dtests.method=testStreamURL -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=de-DE -Dtests.timezone=America/Aruba -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1 [junit4] ERROR 0.01s J2 | SolrRequestParserTest.testStreamURL <<< [junit4] > Throwable #1: org.apache.solr.common.SolrException: Remote Streaming is disabled. [junit4] > at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:BFA5EC71A3C03B8B]:0) [junit4] > at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:190) [junit4] > at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:178) [junit4] > at org.apache.solr.servlet.SolrRequestParserTest.testStreamURL(SolrRequestParserTest.java:140) [...] [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=SolrRequestParserTest -Dtests.method=testStreamFile -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=de-DE -Dtests.timezone=America/Aruba -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1 [junit4] ERROR 0.00s J2 | SolrRequestParserTest.testStreamFile <<< [junit4] > Throwable #1: org.apache.solr.common.SolrException: Remote Streaming is disabled. [junit4] > at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:795923A829FF9159]:0) [junit4] > at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:205) [junit4] > at org.apache.solr.servlet.SolrRequestParsers.buildRequestFrom(SolrRequestParsers.java:178) [junit4] > at org.apache.solr.servlet.SolrRequestParserTest.testStreamFile(SolrRequestParserTest.java:162)
          Hide
          steve_rowe Steve Rowe added a comment -

          git bisect blames the commit on this issue for a reproducing CacheHeaderTest.testCacheVetoHandler() failure https://jenkins.thetaphi.de/job/Lucene-Solr-master-Solaris/1384/:

             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=CacheHeaderTest -Dtests.method=testCacheVetoHandler -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=nl-NL -Dtests.timezone=America/Panama -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
             [junit4] FAILURE 0.04s J0 | CacheHeaderTest.testCacheVetoHandler <<<
             [junit4]    > Throwable #1: java.lang.AssertionError: expected:<200> but was:<400>
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:3636A5FEF7DCEF8F]:0)
             [junit4]    > 	at org.apache.solr.servlet.CacheHeaderTest.testCacheVetoHandler(CacheHeaderTest.java:67)
          
          Show
          steve_rowe Steve Rowe added a comment - git bisect blames the commit on this issue for a reproducing CacheHeaderTest.testCacheVetoHandler() failure https://jenkins.thetaphi.de/job/Lucene-Solr-master-Solaris/1384/ : [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=CacheHeaderTest -Dtests.method=testCacheVetoHandler -Dtests.seed=E690D262D7E2F6BE -Dtests.multiplier=2 -Dtests.slow=true -Dtests.locale=nl-NL -Dtests.timezone=America/Panama -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1 [junit4] FAILURE 0.04s J0 | CacheHeaderTest.testCacheVetoHandler <<< [junit4] > Throwable #1: java.lang.AssertionError: expected:<200> but was:<400> [junit4] > at __randomizedtesting.SeedInfo.seed([E690D262D7E2F6BE:3636A5FEF7DCEF8F]:0) [junit4] > at org.apache.solr.servlet.CacheHeaderTest.testCacheVetoHandler(CacheHeaderTest.java:67)
          Hide
          steve_rowe Steve Rowe added a comment -

          TestRemoteStreaming.testStreamUrl() now fails for me without a seed (for an example, see https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/19916/)

          Show
          steve_rowe Steve Rowe added a comment - TestRemoteStreaming.testStreamUrl() now fails for me without a seed (for an example, see https://jenkins.thetaphi.de/job/Lucene-Solr-master-Linux/19916/ )
          Hide
          janhoy Jan Høydahl added a comment -

          Resolved. Thanks to all who participated, especially Yonik.

          Show
          janhoy Jan Høydahl added a comment - Resolved. Thanks to all who participated, especially Yonik.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4746ff0ec8a008d42a44c6e6dd3e94cbb2886410 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4746ff0 ]

          SOLR-9623: Disable remote streaming in example configs by default. Adjust Upload Limit defaults

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4746ff0ec8a008d42a44c6e6dd3e94cbb2886410 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4746ff0 ] SOLR-9623 : Disable remote streaming in example configs by default. Adjust Upload Limit defaults
          Hide
          janhoy Jan Høydahl added a comment -

          New patch

          • Sets defaults for both multipartUploadLimitInKB and formdataUploadLimitInKB to MAX_INT
          • Allows value "-1" in configs, meaning MAX_INT
          • Updates all solrconfig.xml files with "-1" instead of a number
          • Update ref-guide docs. Let doc example still set a limit of 2048, to highlight example use
          Show
          janhoy Jan Høydahl added a comment - New patch Sets defaults for both multipartUploadLimitInKB and formdataUploadLimitInKB to MAX_INT Allows value "-1" in configs, meaning MAX_INT Updates all solrconfig.xml files with "-1" instead of a number Update ref-guide docs. Let doc example still set a limit of 2048, to highlight example use
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -

          Sorry, this fell off my radar.

          [~yonik] any thought about whether the default limit for formdataUploadLimitInKB should also be raised?

          It should be unlimited.

          The number 2048000 feels a bit like adding zeros to make a big number.

          That's exactly what I did when I hit the magic limit previously... I didn't know the implementation details, and didn't know if this was internally an integer or a long. And without knowing the implementation details, things like MAX_INT or MAX_LONG can cause boundary condition bugs. So I just added some zeroes But if there's a way to make it effectively unlimited, it seems like that's what it should be.

          Show
          yseeley@gmail.com Yonik Seeley added a comment - Sorry, this fell off my radar. [~yonik] any thought about whether the default limit for formdataUploadLimitInKB should also be raised? It should be unlimited. The number 2048000 feels a bit like adding zeros to make a big number. That's exactly what I did when I hit the magic limit previously... I didn't know the implementation details, and didn't know if this was internally an integer or a long. And without knowing the implementation details, things like MAX_INT or MAX_LONG can cause boundary condition bugs. So I just added some zeroes But if there's a way to make it effectively unlimited, it seems like that's what it should be.
          Hide
          janhoy Jan Høydahl added a comment -

          There has been no more feedback on this, so I plan to commit in a few days

          Show
          janhoy Jan Høydahl added a comment - There has been no more feedback on this, so I plan to commit in a few days
          Hide
          janhoy Jan Høydahl added a comment -

          [~yonik] any thought about whether the default limit for formdataUploadLimitInKB should also be raised? As I understand it applies when you post a HTML form or use curl to post without specifying content-type?

          Show
          janhoy Jan Høydahl added a comment - [~yonik] any thought about whether the default limit for formdataUploadLimitInKB should also be raised? As I understand it applies when you post a HTML form or use curl to post without specifying content-type?
          Hide
          janhoy Jan Høydahl added a comment -

          Created SOLR-10748 for the enableStreamBody config.

          Show
          janhoy Jan Høydahl added a comment - Created SOLR-10748 for the enableStreamBody config.
          Hide
          dsmiley David Smiley added a comment -

          Anyone in favor of adding the remoteStreaming check also for stream.body

          Hmm. It seems these are separate concerns. Remote streaming is the concern that you pull from a remote service, and the caller gets to pick the URL which is a security concern. But stream.body is actually related to a GET vs POST issue, which should be handled separately. So I'm -0 on your suggestion.

          All these security checks can be a bit of a downer (depressingly hobbled) for local work. It'd be nice if these checks could be auto-disabled when issues from localhost in non-SolrCloud. Ah well.

          Show
          dsmiley David Smiley added a comment - Anyone in favor of adding the remoteStreaming check also for stream.body Hmm. It seems these are separate concerns. Remote streaming is the concern that you pull from a remote service, and the caller gets to pick the URL which is a security concern. But stream.body is actually related to a GET vs POST issue, which should be handled separately. So I'm -0 on your suggestion. All these security checks can be a bit of a downer (depressingly hobbled) for local work. It'd be nice if these checks could be auto-disabled when issues from localhost in non-SolrCloud. Ah well.
          Hide
          janhoy Jan Høydahl added a comment -

          I was hoping that this would also disallow stream.body but it doesn't. In fact there is no way to disable stream.body. Although useful in tests, I think it is an anti pattern to be able to fake a POST request using GET, which is what this allows in practice.

          Anyone in favor of adding the remoteStreaming check also for stream.body, or alternatively introducing a new requestParsers attribute enableStreamBody which is also false by default?

          Show
          janhoy Jan Høydahl added a comment - I was hoping that this would also disallow stream.body but it doesn't. In fact there is no way to disable stream.body. Although useful in tests, I think it is an anti pattern to be able to fake a POST request using GET, which is what this allows in practice. Anyone in favor of adding the remoteStreaming check also for stream.body , or alternatively introducing a new requestParsers attribute enableStreamBody which is also false by default?
          Hide
          janhoy Jan Høydahl added a comment -

          New patch with new default (2097152) in both Java and configs for multipartUploadLimitKB. What about formdataUploadLimitInKB?

          Show
          janhoy Jan Høydahl added a comment - New patch with new default (2097152) in both Java and configs for multipartUploadLimitKB . What about formdataUploadLimitInKB?
          Hide
          janhoy Jan Høydahl added a comment -

          I follow that logic, to upper the default limit both in Java and in config, now that we disable streaming by default anyway. What is a good value? The number 2048000 feels a bit like adding zeros to make a big number.. Perhaps 2097152 (2048 x 1024) could be the new default I could update this patch to make that change too.

          Show
          janhoy Jan Høydahl added a comment - I follow that logic, to upper the default limit both in Java and in config, now that we disable streaming by default anyway. What is a good value? The number 2048000 feels a bit like adding zeros to make a big number.. Perhaps 2097152 (2048 x 1024) could be the new default I could update this patch to make that change too.
          Hide
          yseeley@gmail.com Yonik Seeley added a comment -

          Changes all multipartUploadLimitInKB="2048000" to 2048

          This seems like it's just one more step to get to useable remote streaming that won't break when it goes over a magic limit. If it's disabled by default, surely we can have a very high limit, or disable the limit altogether by default?

          Show
          yseeley@gmail.com Yonik Seeley added a comment - Changes all multipartUploadLimitInKB="2048000" to 2048 This seems like it's just one more step to get to useable remote streaming that won't break when it goes over a magic limit. If it's disabled by default, surely we can have a very high limit, or disable the limit altogether by default?
          Hide
          janhoy Jan Høydahl added a comment -

          Patch.

          • Flips all configs to "false", in line with code defaults
          • Comments out most <requestParsers> configs
          • Changes all multipartUploadLimitInKB="2048000" to 2048, in line with code defaults
          • Updates ref-guide

          Could be wise to include a Config-API example somewhere in the RefGuide of how to enable streaming

          Show
          janhoy Jan Høydahl added a comment - Patch. Flips all configs to "false", in line with code defaults Comments out most <requestParsers> configs Changes all multipartUploadLimitInKB="2048000" to 2048, in line with code defaults Updates ref-guide Could be wise to include a Config-API example somewhere in the RefGuide of how to enable streaming
          Hide
          janhoy Jan Høydahl added a comment -

          Looking at this again, trying to target 7.0

          I may do this together with SOLR-5077 and adjust both multipartUploadLimitInKB and enableRemoteStreaming in all configs, and comment out the section instead of removing.

          Show
          janhoy Jan Høydahl added a comment - Looking at this again, trying to target 7.0 I may do this together with SOLR-5077 and adjust both multipartUploadLimitInKB and enableRemoteStreaming in all configs, and comment out the section instead of removing.
          Hide
          janhoy Jan Høydahl added a comment - - edited

          You mean to build EnvVar override support into the Java code for each config property instead of relying on solr.xml to have the config? Would be sweet if we had some kind of dot convention that would always work, like solr.<module>.<section>.<property>, e.g. solr.config.request_parsers.enable_remote_streaming, here with a convention of inserting a "_" for camelCase.

          Makes me think - we already have such a mepping convention for the Config REST API, don't we?

          Show
          janhoy Jan Høydahl added a comment - - edited You mean to build EnvVar override support into the Java code for each config property instead of relying on solr.xml to have the config? Would be sweet if we had some kind of dot convention that would always work, like solr.<module>.<section>.<property> , e.g. solr.config.request_parsers.enable_remote_streaming , here with a convention of inserting a "_" for camelCase. Makes me think - we already have such a mepping convention for the Config REST API, don't we?
          Hide
          arafalov Alexandre Rafalovitch added a comment -

          A. Remove this section from all solrconfig.xml and let the Java default of false be the new default

          +1 on A.

          I am -0 on B, because each section in solrconfig.xml also contributes to the decision fatigue. My anecdotal survey indicates that many people don't even know that this section is there because they get tired of reading solrconfig.xml before that due to all other sections.

          Show
          arafalov Alexandre Rafalovitch added a comment - A. Remove this section from all solrconfig.xml and let the Java default of false be the new default +1 on A. I am -0 on B, because each section in solrconfig.xml also contributes to the decision fatigue. My anecdotal survey indicates that many people don't even know that this section is there because they get tired of reading solrconfig.xml before that due to all other sections.
          Hide
          ehatcher Erik Hatcher added a comment -

          ... because it's then easy to enable it with a System property.

          Maybe this could be generalized, such that all settings can correspond to a "solr.some.key" system property override? [maybe with a solr.system.property.resolver=off setting too] Just thinking outloud...

          Show
          ehatcher Erik Hatcher added a comment - ... because it's then easy to enable it with a System property. Maybe this could be generalized, such that all settings can correspond to a "solr.some.key" system property override? [maybe with a solr.system.property.resolver=off setting too] Just thinking outloud...
          Hide
          dsmiley David Smiley added a comment -

          B. Leave the tag in XML files, but change the variable default from true->false

          +1 to 'B' because it's then easy to enable it with a System property. I think this better supports some people getting started with Solr, so that they don't have to go mucking with solrconfig.xml.

          Show
          dsmiley David Smiley added a comment - B. Leave the tag in XML files, but change the variable default from true->false +1 to 'B' because it's then easy to enable it with a System property. I think this better supports some people getting started with Solr, so that they don't have to go mucking with solrconfig.xml.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          A. Remove this section from all solrconfig.xml and let the Java default of false be the new default

          +1

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - A. Remove this section from all solrconfig.xml and let the Java default of false be the new default +1
          Hide
          janhoy Jan Høydahl added a comment -

          Let's have a quick vote shall we. Vetos also welcome at this stage:

          A. Remove this section from all solrconfig.xml and let the Java default of false be the new default
          B. Leave the tag in XML files, but change the variable default from true->false
          C. Leave streaming enabled as is

          My vote: A

          Show
          janhoy Jan Høydahl added a comment - Let's have a quick vote shall we. Vetos also welcome at this stage: A. Remove this section from all solrconfig.xml and let the Java default of false be the new default B. Leave the tag in XML files, but change the variable default from true->false C. Leave streaming enabled as is My vote: A
          Hide
          janhoy Jan Høydahl added a comment -

          You are right that Java code default is false. What I meant is make the example solrconfig's comply with that, so that e.g. data_driven_schema_configs and the other configs either removes the whole section or changes it to false and commented out (for educational reason, like much of the other config).

          I think I'm +1 to removing it from the example configs. You can always call the config API to enable it...

          Show
          janhoy Jan Høydahl added a comment - You are right that Java code default is false. What I meant is make the example solrconfig's comply with that, so that e.g. data_driven_schema_configs and the other configs either removes the whole section or changes it to false and commented out (for educational reason, like much of the other config). I think I'm +1 to removing it from the example configs. You can always call the config API to enable it...
          Hide
          arafalov Alexandre Rafalovitch added a comment -

          I believe it is disabled by default in code (though RefGuide says otherwise). Perhaps we don't need that whole section. People who need it, can find it in the Reference Guide.

          Show
          arafalov Alexandre Rafalovitch added a comment - I believe it is disabled by default in code (though RefGuide says otherwise). Perhaps we don't need that whole section. People who need it, can find it in the Reference Guide.

            People

            • Assignee:
              janhoy Jan Høydahl
              Reporter:
              janhoy Jan Høydahl
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development