Django

Code

Ticket #1278 (closed: fixed)

Opened 5 years ago

Last modified 3 years ago

MEDIA_URL needs to be accessible from templates

Reported by: matt Assigned to: adrian
Milestone: Component: Template system
Version: SVN Keywords:
Cc: adrian, SmileyChris Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

I didn't think of the potential security implications of this until I wrote it up (think {% get_setting DATABASE_PASSWORD %}), but I think it could prove quite useful as a default templatetag.

class SettingNode(Node):
    def __init__(self, varname, setting):
        self.setting = setting
        self.varname = varname

    def render(self, context):
        context[self.varname] = self.setting
        return ""
        
#@register.tag
def get_setting(parser, token):
    """
    Retrieve a setting from the current settings file.  The optional as clause can control
    the variable to be added to the context, otherwise the setting asked for witll be added to
    the context.  For example::
        
        {% get_setting MEDIA_URL [as varname] %}
        
    If the setting does not exist in the current settings file, ``None`` will be returned.  The
    following example will return a bulleted list of installed apps::
    
        {% get_setting INSTALLED_APPS %}
        <ul>
        {% for app in INSTALLED_APPS %}
            <li>{{ app }}</li>
        {% endfor %}
        </ul>   
    """   
    bits = token.contents.split()
    if len(bits) == 2:
        varname = bits[1]
    elif len(bits == 4):
        varname = bits[3]
    else:
        raise TemplateSyntaxError, "'%s' takes either one or three arguments." % bits[0]
    try:
        setting = bits[1]
        imp = __import__('django.conf.settings', locals(), globals(), bits[1])
        retsetting = getattr(imp, bits[1])
    except AttributeError:
        retsetting = None
    return SettingNode(varname, retsetting)

get_setting = register.tag('get_setting', get_setting)

Attachments

template_context_constants.patch (3.2 kB) - added by SmileyChris on 02/19/07 17:05:43.
patch with tests
template_context_constants.2.patch (3.1 kB) - added by SmileyChris on 02/19/07 17:06:42.
tiny fix
1278.diff (2.8 kB) - added by ubernostrum on 02/21/07 12:39:57.
Patch adding a 'media' context processor and associated documentation

Change History

01/26/06 11:07:27 changed by adrian

I don't think it should be a default tag, but maybe we can put it contrib for people who want it.

01/26/06 13:06:43 changed by matt

Here's a modified version that uses get_safe_settings from django.views.debug . It would probably be best to move get_safe_settings to somewhere in django.utils and call that from both the debug view and this templatetag.

from django.views.debug import get_safe_settings
class SettingNode(Node):
    def __init__(self, varname, setting):
        self.setting = setting
        self.varname = varname

    def render(self, context):
        context[self.varname] = self.setting
        return ""
        
#@register.tag
def get_setting(parser, token):
    """
    Retrieve a setting from the current settings file.  The optional as clause can control
    the variable to be added to the context, otherwise the setting asked for witll be added to
    the context.  For example::
        
        {% get_setting MEDIA_URL [as varname] %}
        
    If the setting does not exist in the current settings file, ``None`` will be returned.  The
    following example will return a bulleted list of installed apps::
    
        {% get_setting INSTALLED_APPS %}
        <ul>
        {% for app in INSTALLED_APPS %}
            <li>{{ app }}</li>
        {% endfor %}
        </ul>   
    """   
    bits = token.contents.split()
    if len(bits) == 2:
        varname = bits[1]
    elif len(bits) == 4:
        varname = bits[3]
    else:
        raise TemplateSyntaxError, "'%s' takes either one or three arguments." % bits[0]
    try:
        settings = get_safe_settings()
        retsetting = settings[bits[1]]
    except KeyError:
        retsetting = None
    return SettingNode(varname, retsetting)

get_setting = register.tag('get_setting', get_setting)

04/23/06 18:10:03 changed by adrian

  • priority changed from normal to lowest.
  • severity changed from normal to minor.

05/26/06 22:20:38 changed by anonymous

  • priority changed from lowest to normal.

This would be very useful when developing - getting rid of hardcoded media paths being the major example. It's easy to pass <project>.settings to views in render and use settings.MEDIA_URL, but you can't do this for generic views.

07/21/06 10:42:46 changed by adrian

  • status changed from new to closed.
  • resolution set to wontfix.

Note that this tag can be written a lot more easily now with the simple_tag decorator.

And I'm marking it as a wontfix because if it goes into trunk, it'd be too easy for a template author to get sensitive settings such as the database password.

02/19/07 16:32:06 changed by Simon Litchfield <simon@slicmedia.com>

  • cc set to adrian.
  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from Potentially useful get_setting templatetag to MEDIA_URL needs to be accessible from templates.

As a new user, I was shocked and frustrated to find that MEDIA_URL wasn't available in templates. And that you needed to write into every context, or make a custom tag, or context_processor just to get it! It's a seriously painful unnecessary complication and stumbling block for new users. Now I've got RequestContext? crap all through my code for no good reason at all. What happened to DRY, less code is better? 99% of templates need access to this setting.

Obviously we don't want template authors having access to all settings though. Goes without saying.

Can I suggest either we select a few fundamentally important settings for template authors, and make them available via {{ settings.MEDIA_URL }} or even simply {{ MEDIA_URL }}. Or perhaps better still, to make it more self-explanatory, maybe we define a list in settings.py --

Example --

TEMPLATE_SETTINGS = ['MEDIA_URL', 'GOOGLE_MAPS_KEY', 'SOME_OTHER_SETTING']

Or, if we prefer to be more specific --

TEMPLATE_CONTEXT_CONSTANTS = {'MEDIA_URL': MEDIA_URL, 'GOOGLE_MAPS_KEY': GOOGLE_MAPS_KEY ... }

02/19/07 16:44:11 changed by ubernostrum

Even if Django shipped a built-in context processor for retrieving some settings, you'd still need to use RequestContext to get them to show up in your template contexts; there's no way around that. I do think there's utility in deciding on a set of default settings like MEDIA_URL, etc. which are safe and which could have a built-in context processor.

Adding things to template contexts which are specific to the needs of your application is the job of custom context processors in your application; that's explicitly what context processors and RequestContext are designed for.

02/19/07 16:55:05 changed by Simon Litchfield <simon@slicmedia.com>

Agreed, but context processors and RequestContext? are overly and needlessly clunky for settings like MEDIA_URL which, lets be realistic for a second, are needed in 99.9% of all half serious web sites.

This is definitely only one of those 'little things'; but, it's one of those little things that's important, particularly for new users, and needs to be done right. See my post above for some suggestions on how to handle.

Come on people, using MEDIA_URL is not an exception here at all, it's the rule; and a good practice that should be encouraged by the framework right from square one.

02/19/07 17:00:49 changed by adrian

  • status changed from reopened to closed.
  • resolution set to wontfix.

I don't buy the argument that 99.9% of all half serious Web sites "need" this setting. I haven't needed it in years.

Marking it as a wontfix, again.

02/19/07 17:05:43 changed by SmileyChris

  • attachment template_context_constants.patch added.

patch with tests

02/19/07 17:06:42 changed by SmileyChris

  • attachment template_context_constants.2.patch added.

tiny fix

02/19/07 17:12:55 changed by SmileyChris

For posterity, here is Simon's idea as a patch (which should have read "patch with docs").

Adrian, how do you link to your CSS in a template correctly without providing MEDIA_URL (or STATIC_URL, etc) to your templates or (ugh) hard-coding it?

02/19/07 18:32:09 changed by Simon Litchfield <simon@slicmedia.com>

  • status changed from closed to reopened.
  • resolution deleted.

Yes yes, enlighten us, how do we link media without MEDIA_URL? I see the reason for wontfix last time was security. That's pretty obvious. Whats the reason this time?

02/19/07 18:34:07 changed by Simon Litchfield <simon@slicmedia.com>

Nice work Smiley, your patch rocks :-)

02/19/07 18:43:01 changed by adrian

  • status changed from reopened to closed.
  • resolution set to wontfix.

Hi Simon: My reasoning is, it's feature creep.

Please don't reopen this ticket. I've marked it as a wontfix.

02/19/07 18:50:10 changed by Simon Litchfield <simon@slicmedia.com>

  • cc changed from adrian to adrian, SmileyChris.

Hey Adrian. Sorry to be a pain! Lets assume for a second it's feature creep. The question remains. How do we link media without MEDIA_URL?

02/19/07 18:52:36 changed by SmileyChris

Simon, you may be better off taking this discussion to the django developers group.

02/20/07 15:42:06 changed by anonymous

I don't really see a need for a {% media_url %} either. The get_img_tag method figures out by itself where the static dir is. If you need other methods to figure it out, it's easy too.

02/20/07 16:13:50 changed by Simon Litchfield <simon@slicmedia.com>

Not talking about imagefields and the like here, this is for all your flat files like css, logos/interface images, stock js's, etc. Hard-coding those into templates doesn't make sense; the framework encourages pluggability and separated delivery.

Anyway agreed the _constants idea is creep; _processors already handles that.

Still think MEDIA_URL should be a core tag, or made available through a default context_processor though, or something like that; so newbies in particular are encouraged to use it straight up. Theres numerous blogs and such out there showing workarounds, and plenty of queries have come in over it...

02/21/07 12:39:57 changed by ubernostrum

  • attachment 1278.diff added.

Patch adding a 'media' context processor and associated documentation

05/03/07 09:43:01 changed by Marc Fargas <telenieko@telenieko.com>

  • stage changed from Unreviewed to Design decision needed.

05/29/07 05:52:22 changed by russellm

  • status changed from closed to reopened.
  • resolution deleted.

Putting MEDIA_URL in RequestContext? has been blessed by Adrian.

05/29/07 06:07:44 changed by Marc Fargas <telenieko@telenieko.com>

  • has_patch set to 1.
  • stage changed from Design decision needed to Accepted.

05/29/07 06:09:24 changed by russellm

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [5379]) Fixed #1278 -- Added a context processor that puts MEDIA_URL in the context, and added that context processor to the default set. Thanks to Ubernostrum for the patch.


Add/Change #1278 (MEDIA_URL needs to be accessible from templates)




Change Properties
Action