[MGNLDIFF-68] Make socket type configurable plain vs. SSL Created: 02/Dec/13  Updated: 03/Mar/17  Resolved: 03/Mar/17

Status: Closed
Project: Magnolia Diff Module
Component/s: None
Affects Version/s: 1.1.5
Fix Version/s: 1.2.1, 1.5.2

Type: Improvement Priority: Neutral
Reporter: Marcel Stör Assignee: Jaroslav Simak
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:
Visible to:
Zarko Ivanoski

 Description   

MGNLDIFF-30 added config options. What's missing is a flag for the socket type plain vs. SSL.

The problems this may cause are described in a comment of SUPPORT-2971: http://jira.magnolia-cms.com/browse/SUPPORT-2971?focusedCommentId=74604&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-74604



 Comments   
Comment by Philip Mundt [ 03/Jan/14 ]

Getter and setter in module class cannot be "uppercased", otherwise Node2Bean can't populate module instance properly.

Comment by Marcel Stör [ 23/Apr/14 ]

I have two issues with this fix:

  • The new property ssl doesn't seem to be documented anywhere
  • VersionDiffHtmlServlet.setHostConfiguration(HttpClient, String) uses an OR-condition when an AND-condition should IMO be used. If I configure ssl=false then I don't want it to be used regardless of the protocol of the original request URL
    PROTOCOL_HTTPS.equals(protocol) || diffModule.isSsl()

    If the condition is updated the default value for ssl would have to be set to true.

Comment by Marcel Stör [ 25/Apr/14 ]

In MGNLDIFF-79 pmundt asked:

So in your setup scenario you don't want to use SSL for the diff even though Magnolia is accessed by HTTPS. That's why you would like to have the AND connection of the conditions!?

No, this issue is only slightly related to MGNLDIFF-79. It's a question of "reasonable expectation".
If there's a config option that allows you to enable or disable SSL and you set ssl=false wouldn't you expect that SSL is not used no matter what? I think it's odd when Magnolia asks me to decide whether I want to use SSL and if I say "no" it still uses SSL.

Comment by Philip Mundt [ 25/Apr/14 ]

I would agree. To summarize:

  1. ssl=false (no SSL should be used)
  2. ssl=true (SSL is used)
  3. ssl=unspecified, but protocol=HTTPS (SSL is used) (I just realized that this case won't work, as by default ssl is indeed false (i.e. this would break backwards-compatibility for users that haven't set the flag but are using HTTPS))
Comment by Marcel Stör [ 28/Apr/14 ]

ssl=unspecified, but protocol=HTTPS (SSL is used) (I just realized that this case won't work, as by default ssl is indeed false

I prefer to use Boolean, which may be null, over boolean exactly because of cases like this one.

Comment by Philip Mundt [ 28/Apr/14 ]

This will indeed work, as Node2Bean will set the value to null, if no property exists in JCR.

Generated at Mon Feb 12 05:21:04 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.