You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by mosermw <gi...@git.apache.org> on 2016/05/31 13:50:47 UTC

[GitHub] nifi pull request: NIFI-1937 GetHTTP configurable redirect cookie policy

GitHub user mosermw opened a pull request:

    https://github.com/apache/nifi/pull/479

    NIFI-1937 GetHTTP configurable redirect cookie policy

    NIFI-1937 allows GetHTTP to configure a cookie policy in Apache HTTPClient to use for redirects.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mosermw/nifi nifi-1937

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/479.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #479
    
----
commit 40d695d8df5b9f504a93bfd9e97bb93600b9d75c
Author: Mike Moser <mo...@apache.org>
Date:   2016-05-27T21:11:10Z

    NIFI-1937 GetHTTP configurable redirect cookie policy

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #479: NIFI-1937 GetHTTP configurable redirect cookie polic...

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/479#discussion_r66706057
  
    --- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java ---
    @@ -197,6 +198,30 @@
                 .addValidator(StandardValidators.PORT_VALIDATOR)
                 .build();
     
    +    public static final String DEFAULT_COOKIE_POLICY_STR = "default";
    +    public static final String STANDARD_COOKIE_POLICY_STR = "standard";
    +    public static final String STRICT_COOKIE_POLICY_STR = "strict";
    +    public static final String NETSCAPE_COOKIE_POLICY_STR = "netscape";
    +    public static final String IGNORE_COOKIE_POLICY_STR = "ignore";
    +    public static final AllowableValue DEFAULT_COOKIE_POLICY = new AllowableValue(DEFAULT_COOKIE_POLICY_STR, DEFAULT_COOKIE_POLICY_STR,
    +            "Default cookie policy that provides a higher degree of compatibility with common cookie management of popular HTTP agents for non-standard (Netscape style) cookies.");
    +    public static final AllowableValue STANDARD_COOKIE_POLICY = new AllowableValue(STANDARD_COOKIE_POLICY_STR, STANDARD_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (interoperability profile).");
    +    public static final AllowableValue STRICT_COOKIE_POLICY = new AllowableValue(STRICT_COOKIE_POLICY_STR, STRICT_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (strict profile).");
    +    public static final AllowableValue NETSCAPE_COOKIE_POLICY = new AllowableValue(NETSCAPE_COOKIE_POLICY_STR, NETSCAPE_COOKIE_POLICY_STR,
    +            "Netscape draft compliant cookie policy.");
    +    public static final AllowableValue IGNORE_COOKIE_POLICY = new AllowableValue(IGNORE_COOKIE_POLICY_STR, IGNORE_COOKIE_POLICY_STR,
    +            "A cookie policy that ignores cookies.");
    +
    +    public static final PropertyDescriptor REDIRECT_COOKIE_POLICY = new PropertyDescriptor.Builder()
    +            .name("redirect-cookie-policy")
    +            .displayName("Redirect Cookie Policy")
    +            .description("When a HTTP server responds to a request with a redirect, this is the cookie policy used to copy cookies to the following request.")
    +            .allowableValues(DEFAULT_COOKIE_POLICY, STANDARD_COOKIE_POLICY, STRICT_COOKIE_POLICY, NETSCAPE_COOKIE_POLICY, IGNORE_COOKIE_POLICY)
    +            .defaultValue(DEFAULT_COOKIE_POLICY_STR)
    --- End diff --
    
    @trkurc thanks for reviewing.  Before version 0.6.0 we didn't specify a cookie spec, so it was DEFAULT.  I made a change in 0.6.0 to use CookieSpecs.STANDARD, thinking this just increased compatibility with more web sites.  When I found that was not true, I suggested via this PR that we make the CookieSpecs configurable in the processor.  So using .defaultValue(DEFAULT_COOKIE_POLICY_STR) here takes us back to the default value pre 0.6.0.
    
    I was hoping that it's rare that a cookie policy matters at all to GetHTTP, so taking us back to pre 0.6.0 functionality, by default, would be OK if not the desired functionality.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #479: NIFI-1937 GetHTTP configurable redirect cookie polic...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/479


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #479: NIFI-1937 GetHTTP configurable redirect cookie policy

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on the issue:

    https://github.com/apache/nifi/pull/479
  
    I'll review this one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #479: NIFI-1937 GetHTTP configurable redirect cookie polic...

Posted by mosermw <gi...@git.apache.org>.
Github user mosermw commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/479#discussion_r66706268
  
    --- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java ---
    @@ -197,6 +198,30 @@
                 .addValidator(StandardValidators.PORT_VALIDATOR)
                 .build();
     
    +    public static final String DEFAULT_COOKIE_POLICY_STR = "default";
    +    public static final String STANDARD_COOKIE_POLICY_STR = "standard";
    +    public static final String STRICT_COOKIE_POLICY_STR = "strict";
    +    public static final String NETSCAPE_COOKIE_POLICY_STR = "netscape";
    +    public static final String IGNORE_COOKIE_POLICY_STR = "ignore";
    +    public static final AllowableValue DEFAULT_COOKIE_POLICY = new AllowableValue(DEFAULT_COOKIE_POLICY_STR, DEFAULT_COOKIE_POLICY_STR,
    +            "Default cookie policy that provides a higher degree of compatibility with common cookie management of popular HTTP agents for non-standard (Netscape style) cookies.");
    +    public static final AllowableValue STANDARD_COOKIE_POLICY = new AllowableValue(STANDARD_COOKIE_POLICY_STR, STANDARD_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (interoperability profile).");
    +    public static final AllowableValue STRICT_COOKIE_POLICY = new AllowableValue(STRICT_COOKIE_POLICY_STR, STRICT_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (strict profile).");
    +    public static final AllowableValue NETSCAPE_COOKIE_POLICY = new AllowableValue(NETSCAPE_COOKIE_POLICY_STR, NETSCAPE_COOKIE_POLICY_STR,
    +            "Netscape draft compliant cookie policy.");
    +    public static final AllowableValue IGNORE_COOKIE_POLICY = new AllowableValue(IGNORE_COOKIE_POLICY_STR, IGNORE_COOKIE_POLICY_STR,
    +            "A cookie policy that ignores cookies.");
    +
    +    public static final PropertyDescriptor REDIRECT_COOKIE_POLICY = new PropertyDescriptor.Builder()
    +            .name("redirect-cookie-policy")
    +            .displayName("Redirect Cookie Policy")
    +            .description("When a HTTP server responds to a request with a redirect, this is the cookie policy used to copy cookies to the following request.")
    +            .allowableValues(DEFAULT_COOKIE_POLICY, STANDARD_COOKIE_POLICY, STRICT_COOKIE_POLICY, NETSCAPE_COOKIE_POLICY, IGNORE_COOKIE_POLICY)
    +            .defaultValue(DEFAULT_COOKIE_POLICY_STR)
    --- End diff --
    
    If you have the time, please go ahead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #479: NIFI-1937 GetHTTP configurable redirect cookie polic...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/479#discussion_r66706187
  
    --- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java ---
    @@ -197,6 +198,30 @@
                 .addValidator(StandardValidators.PORT_VALIDATOR)
                 .build();
     
    +    public static final String DEFAULT_COOKIE_POLICY_STR = "default";
    +    public static final String STANDARD_COOKIE_POLICY_STR = "standard";
    +    public static final String STRICT_COOKIE_POLICY_STR = "strict";
    +    public static final String NETSCAPE_COOKIE_POLICY_STR = "netscape";
    +    public static final String IGNORE_COOKIE_POLICY_STR = "ignore";
    +    public static final AllowableValue DEFAULT_COOKIE_POLICY = new AllowableValue(DEFAULT_COOKIE_POLICY_STR, DEFAULT_COOKIE_POLICY_STR,
    +            "Default cookie policy that provides a higher degree of compatibility with common cookie management of popular HTTP agents for non-standard (Netscape style) cookies.");
    +    public static final AllowableValue STANDARD_COOKIE_POLICY = new AllowableValue(STANDARD_COOKIE_POLICY_STR, STANDARD_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (interoperability profile).");
    +    public static final AllowableValue STRICT_COOKIE_POLICY = new AllowableValue(STRICT_COOKIE_POLICY_STR, STRICT_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (strict profile).");
    +    public static final AllowableValue NETSCAPE_COOKIE_POLICY = new AllowableValue(NETSCAPE_COOKIE_POLICY_STR, NETSCAPE_COOKIE_POLICY_STR,
    +            "Netscape draft compliant cookie policy.");
    +    public static final AllowableValue IGNORE_COOKIE_POLICY = new AllowableValue(IGNORE_COOKIE_POLICY_STR, IGNORE_COOKIE_POLICY_STR,
    +            "A cookie policy that ignores cookies.");
    +
    +    public static final PropertyDescriptor REDIRECT_COOKIE_POLICY = new PropertyDescriptor.Builder()
    +            .name("redirect-cookie-policy")
    +            .displayName("Redirect Cookie Policy")
    +            .description("When a HTTP server responds to a request with a redirect, this is the cookie policy used to copy cookies to the following request.")
    +            .allowableValues(DEFAULT_COOKIE_POLICY, STANDARD_COOKIE_POLICY, STRICT_COOKIE_POLICY, NETSCAPE_COOKIE_POLICY, IGNORE_COOKIE_POLICY)
    +            .defaultValue(DEFAULT_COOKIE_POLICY_STR)
    --- End diff --
    
    Okay, based on that description I am +1 on the pr. Would you like to merge, or shall I?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #479: NIFI-1937 GetHTTP configurable redirect cookie polic...

Posted by trkurc <gi...@git.apache.org>.
Github user trkurc commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/479#discussion_r66699726
  
    --- Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java ---
    @@ -197,6 +198,30 @@
                 .addValidator(StandardValidators.PORT_VALIDATOR)
                 .build();
     
    +    public static final String DEFAULT_COOKIE_POLICY_STR = "default";
    +    public static final String STANDARD_COOKIE_POLICY_STR = "standard";
    +    public static final String STRICT_COOKIE_POLICY_STR = "strict";
    +    public static final String NETSCAPE_COOKIE_POLICY_STR = "netscape";
    +    public static final String IGNORE_COOKIE_POLICY_STR = "ignore";
    +    public static final AllowableValue DEFAULT_COOKIE_POLICY = new AllowableValue(DEFAULT_COOKIE_POLICY_STR, DEFAULT_COOKIE_POLICY_STR,
    +            "Default cookie policy that provides a higher degree of compatibility with common cookie management of popular HTTP agents for non-standard (Netscape style) cookies.");
    +    public static final AllowableValue STANDARD_COOKIE_POLICY = new AllowableValue(STANDARD_COOKIE_POLICY_STR, STANDARD_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (interoperability profile).");
    +    public static final AllowableValue STRICT_COOKIE_POLICY = new AllowableValue(STRICT_COOKIE_POLICY_STR, STRICT_COOKIE_POLICY_STR,
    +            "RFC 6265 compliant cookie policy (strict profile).");
    +    public static final AllowableValue NETSCAPE_COOKIE_POLICY = new AllowableValue(NETSCAPE_COOKIE_POLICY_STR, NETSCAPE_COOKIE_POLICY_STR,
    +            "Netscape draft compliant cookie policy.");
    +    public static final AllowableValue IGNORE_COOKIE_POLICY = new AllowableValue(IGNORE_COOKIE_POLICY_STR, IGNORE_COOKIE_POLICY_STR,
    +            "A cookie policy that ignores cookies.");
    +
    +    public static final PropertyDescriptor REDIRECT_COOKIE_POLICY = new PropertyDescriptor.Builder()
    +            .name("redirect-cookie-policy")
    +            .displayName("Redirect Cookie Policy")
    +            .description("When a HTTP server responds to a request with a redirect, this is the cookie policy used to copy cookies to the following request.")
    +            .allowableValues(DEFAULT_COOKIE_POLICY, STANDARD_COOKIE_POLICY, STRICT_COOKIE_POLICY, NETSCAPE_COOKIE_POLICY, IGNORE_COOKIE_POLICY)
    +            .defaultValue(DEFAULT_COOKIE_POLICY_STR)
    --- End diff --
    
    I was just about to merge this in, and realized that we might want to have a different default for 0.x and 1.x. 0.x prior to this had CookieSpecs.STANDARD set, and this would potentially change behavior without reconfiguring a flow back to standard, but admittedly, I had no success making a test that worked with CookieSpecs.STANDARD and broke with CookieSpecs.DEFAULT. @mosermw  - how do you feel about .defaultValue(STANDARD_COOKIE_POLICY_STR) in 0.x for flow compatibility versus writing a migration note, and .defaultValue(DEFAULT_COOKIE_POLICY_STR) in 1.x which is what I think a reasonable default where flow compatibility is less an issue?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---