Moodle
  1. Moodle
  2. MDL-28484

We do not have a function to detect if we are being served on https

    Details

    • Type: Bug Bug
    • Status: Development in progress
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1, 2.5.2
    • Fix Version/s: STABLE backlog, BACKEND
    • Component/s: Libraries
    • Labels:
    • Rank:
      18142

      Description

      As we don't have a function, different places around Moodle are all doing a string comparison themselves.

        Issue Links

          Activity

          Dan Poltawski created issue -
          Dan Poltawski made changes -
          Field Original Value New Value
          Assignee moodle.com [ moodle.com ] Dan Poltawski [ poltawski ]
          Dan Poltawski made changes -
          Status Open [ 1 ] Development in progress [ 3 ]
          Hide
          Dan Poltawski added a comment -

          Hi Petr,

          You are are the resident expert on these https issues, I was wondering what you thought about this?

          (I don't like the name of my moodlelib function and I also don't know if moodlelib is the right place for this).

          The use case is that in a plugin I want to embed some content from an external site - and convert the (external) url to https:// if moodle is served on https - to prevent 'insecure content' warnings in IE

          Show
          Dan Poltawski added a comment - Hi Petr, You are are the resident expert on these https issues, I was wondering what you thought about this? (I don't like the name of my moodlelib function and I also don't know if moodlelib is the right place for this). The use case is that in a plugin I want to embed some content from an external site - and convert the (external) url to https:// if moodle is served on https - to prevent 'insecure content' warnings in IE
          Dan Poltawski made changes -
          Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
          Peer reviewer skodak
          Hide
          Dan Poltawski added a comment -

          An example of a use for this is in MDL-28486

          Show
          Dan Poltawski added a comment - An example of a use for this is in MDL-28486
          Dan Poltawski made changes -
          Link This issue blocks MDL-28486 [ MDL-28486 ]
          Hide
          Petr Škoda added a comment -

          Thinking aloud - hmm, there are two alternative locations:
          1/ setuplib.php - it verifies the HTTP_HOST and sets up the FULLME, ME and friends
          2/ $PAGE->https_required() does something related, the problem is that the PAGE should not be used everywhere (I am not really sure about filelib.php now)

          The linked MDL->28486 should probably use something like PAGE->is_https() because it should probably deal with loginhttps too in case somebody uses youtube on the login/profile page.

          Show
          Petr Škoda added a comment - Thinking aloud - hmm, there are two alternative locations: 1/ setuplib.php - it verifies the HTTP_HOST and sets up the FULLME, ME and friends 2/ $PAGE->https_required() does something related, the problem is that the PAGE should not be used everywhere (I am not really sure about filelib.php now) The linked MDL->28486 should probably use something like PAGE->is_https() because it should probably deal with loginhttps too in case somebody uses youtube on the login/profile page.
          Hide
          Petr Škoda added a comment -

          Hmm, we should really get rid of the text caching we have in format_text and pass the PAGE as filter parameter instead. The plugins that need access to db could cache only the necessary information more effectively. There is a bug for this somewhere, I can not find it now, sorry...

          Show
          Petr Škoda added a comment - Hmm, we should really get rid of the text caching we have in format_text and pass the PAGE as filter parameter instead. The plugins that need access to db could cache only the necessary information more effectively. There is a bug for this somewhere, I can not find it now, sorry...
          Michael de Raadt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Labels triaged
          Hide
          Rajesh Taneja added a comment -

          Hello Dan,

          You might want to update all instances of https detection with added function.

          Show
          Rajesh Taneja added a comment - Hello Dan, You might want to update all instances of https detection with added function.
          Rajesh Taneja made changes -
          Link This issue has a non-specific relationship to MDL-27364 [ MDL-27364 ]
          Dan Poltawski made changes -
          Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
          Hide
          Andrew Nicols added a comment -

          Petr:

          I'm looking at this again in combination with an updated patch for MDL-28486 (youtube/vimeo).

          I was pondering between:

          • lib/pagelib.php
          • lib/weblib.php
          • lib/setuplib.php

          With pagelib, I agree that it should be on a per-page basis. The difficulty here comes in making sure that we're not duplicating code or forgetting scenarios (e.g. checking that login, profile, and admin pages return the correct value based on loginhttps).

          I also considered using setuplib to determine the correct return value and adding a function to weblib – the checks for sslproxy and friends are all located in the initialise_fullme() function. It would be pretty simple to check the value of $rurl['scheme'] but it feels a touch wrong. Additionally, I'm not sure of the best place to store the value - $CFG feels wrong too.

          Have you got any thoughts on this?

          TIA,

          Andrew

          Show
          Andrew Nicols added a comment - Petr: I'm looking at this again in combination with an updated patch for MDL-28486 (youtube/vimeo). I was pondering between: lib/pagelib.php lib/weblib.php lib/setuplib.php With pagelib, I agree that it should be on a per-page basis. The difficulty here comes in making sure that we're not duplicating code or forgetting scenarios (e.g. checking that login, profile, and admin pages return the correct value based on loginhttps). I also considered using setuplib to determine the correct return value and adding a function to weblib – the checks for sslproxy and friends are all located in the initialise_fullme() function. It would be pretty simple to check the value of $rurl ['scheme'] but it feels a touch wrong. Additionally, I'm not sure of the best place to store the value - $CFG feels wrong too. Have you got any thoughts on this? TIA, Andrew
          Hide
          Petr Škoda added a comment -

          hi

          1/ it should go to weblib.php
          2/ I guess it should return true on loginhttps pages too

          Show
          Petr Škoda added a comment - hi 1/ it should go to weblib.php 2/ I guess it should return true on loginhttps pages too
          Dan Poltawski made changes -
          Link This issue has been marked as being related by MDL-38837 [ MDL-38837 ]
          Dan Poltawski made changes -
          Fix Version/s BACKEND [ 12582 ]
          Dan Poltawski made changes -
          Affects Version/s 2.5.2 [ 12664 ]

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: