Add fingerprinting META Generator and HTTP headers for Drupal

Not a bad idea, lots of other systems do it, and it would increase exposure (a long previous issue about hiding version and fingerprinting data as a security precaution notwithstanding).

There are some minor code style issues -- comments that need to be full sentences complete with periods, and I'm not convinced, to use the classic example, that my mother would understand the form description "Choose whether your HTML should show a META tag with the GENERATOR as Drupal. You can choose whether to display the major version (e.g. "Drupal 7"), the major and minor versions (e.g. "Drupal 7.2"), no version (only "Drupal"), or nothing at all." I think descriptions like this should not only point out that you can choose -- that part is obvious -- but perhaps give some advice about why one may wish to do so.

In terms of advertising for Drupal, this is roughly equivalent to adding the Drupal banner block, which I also thought was a good move.

And there is a patch, so I'm going for code needs work.

Sounds useful to me.

I'm not sure I should expose this level of configuration to the user though. It seems to me like 99.9% of the users wouldn't care about this to begin with. I'd simply make it a theme thing to take it out, and hard-code a specific format. Let's make Drupal easier to use. The last thing my dad wants to do is learn about the generator-tag.

I'm OK with a variable, but personally, I think it is overkill still. It can be removed at the theme-level.

Let's also make sure to write a test for this.

I still think that adding a setting for this is overkill, but I'll let others comment on this.

I am curious why #5 uses node_init() to add the generator text. Wouldn't system_init() make more sense? It doesn't really have anything to do with nodes.

I don't have any problem with the concept of the patch itself.

CNW: Extra newline above function node_last_changed and I agree with Crell that this should be in system rather than node.

In general, I'm +1 for the feature, but feel that the comment block in settings.php is overkill.

The two other variables without a UI in core are dev_query and allow_insecure_uploads. These have basically no documentation within core itself. If this new feature were documented in a handbook page that would seem reasonable to me.

For a themeable solution drupal_meta_generator can call a theme function, right? That would allow overriding at the theme layer.

+  drupal_set_header(theme('generator_header', $version));
+  drupal_set_html_head(theme('generator_head($version));

Then those two theme_ functions return the properly formatted string. It also fits with the goal of no hard-coded html in core.

Why is it

list($major,) = explode('.', VERSION);
$version = $major;

instead of

list($version,) = explode('.', VERSION);

?

Installed the patch, but I see no HTTP header nor meta tag. Do I need to clear a cache or reinstall the site?

Edit: Gah, sorry, I missed the fact that this is done with theme functions. Theme registry rebuilt.

It's functional and adheres to code style. There is one typo in hook_theme, namely 'arguments' => array('version' => NULL,), where the last comma should be outside the parenthesis.

This patch fixes the typo.

I've committed this to CVS HEAD. I'm marking it 'code needs work' because it would be good to have a test case for this.

Thanks all.

If we can turn this thing off, I'm OK with "Generator".

It's not because a few CMS advertise themselves in the HTTP headers that Drupal should do the same. Pulling security issues under the rug won't help either. With generator on, it will be a lot easier for hacker to target security holes with much more efficiency.

So, ultimatly, the decision should be made at the site admin level. We must not forget that there are a lot of corporate users using Drupal for extranet and they would want to make sure this is turned off.

Very cool. The only things I see are periods at the ends of a few comments at the ends of periods and consistent use of either "meta", "Meta", (or presumably, "META").

Committed patch #22 to CVS HEAD. Thanks kbahey.

I’m testing this new feature. Now each page have:

<meta name="Generator" content="Drupal 7 (http://drupal.org)" />

Since this is a theme function, it can be overridden like any other theme function with a few lines of code.

Can you give me an example of the few lines of code I would need to either remove the Generator completly or
be more specific in the version number? Would we be able to display "Drupal 7.0-dev" instead of the major version number?

Overriding the HTML meta works perfectly. Please ignore previous request.

This broke poll.test

Marking #276200 - "poll tests don't pass" postponed until this is resolved.

edit: I doubled checked, and just to confirm, the poll theming is actually broken, not just the test.

... I see that even with unit testing, the poll module still retains its status as the catcher of obscure regressions!

However, I must come to the defense of this issue. It is innocent of the crime. Compare:

The output of the theme call:

<span id="thmr_34" class="thmr_call">

<div class="text">simpletest_acn6</div>
<div class="bar">
  <div style="width: 0%;" class="foreground"></div>
</div>
<div class="percent">
  0% (0 votes)
</div>
</span>

The page that was returned to CURL:

<div class="text">simpletest_P1sy</div>
<div class="bar">
  <div style="width: 0%;" class="foreground"></div>
</div>
<div class="percent">
  0% (0 votes)
</div>

<div class="text">simpletest_iw8Q</div>
<div class="bar">
  <div style="width: 0%;" class="foreground"></div>
</div>
<div class="percent">
  0% (0 votes)
</div>

<div class="text">simpletest_acn6</div>
<div class="bar">
  <div style="width: 0%;" class="foreground"></div>
</div>
<div class="percent">
  0% (0 votes)
</div>
  <div class="total">
    Total votes: 0  </div>
  </div>
  </div>

I accuse devel_themer, with the span-element, in the theming layer!

I can confirm that after disabling devel_themer, poll.test passes. I'll open a new issue.

Test fails without theme developer for me, as does looking at a poll with my own eyes the old fashioned way.

Which tests fail for you? In my case, with theme developer enabled, only the 7 "Choice bar is themed" tests failed, and they passed when I disabled theme developer.

The poll bars don't look right, but that's a CSS problem and it is also caused by theme developer.

Whoops.

Run all tests - poll has 7 failures.
Run poll test by itself - all passes.

With or without this patch.

Of course I was running all tests on HEAD, and narrowed down to poll test when trying to find why it was breaking. Sorry.

oups, back to previous status ...

I can confirm that this patch breaks the block module in the block_admin_display() function and any module that tries to change the theme using $custom_theme variable.

Bug background:
1) enable two themes in theme settings
2) go to admin/build/block
3) click the local menu task to change blocks for a theme which is not the default (active) theme. For example if foo_theme is your active theme click on /admin/build/block/list/bar_theme
4) the list of blocks enable displays for the default active foo_theme, instead of the bar_theme that the user selected.
5) saved changes affect only the default theme and not the selcted theme

The block module tries to change the theme to bar_theme, but cannot because the theme() calls in system_init have already initialized the theme (I found this using var_dump(debug_backtrace());
in init_theme()). The problem comes from calling theme() functions too early in the init process. After these lines in this patch:

// Emit the META tag in the HTML HEAD section
theme('meta_generator_html', $version);

// Emit the HTTP Header too
theme('meta_generator_header', $version)

No other module can change the theme by settings the $custom_theme variable. This bug is not present in D6 where these theme() lines are not present.

Please see the discussion on the dev mailing list called "$custom_theme when does it take effect... when does it not" at http://lists-new.drupal.org/pipermail/development/2008-August/030893.html

Marking as critical since blocks admin is properly broken.

What I hoped can't be fixed. But this bug can...

Edit. testing instructions: enable Marvin and go to admin/build/block/list/marvin . Observe the Garland theme without patch and Marvin with the patch.

On a related note, we need a test case to verify that the $custom_theme hack works, and continues to work. Would be great if that could be part of this patch. It is the perfect test case for it, it seems.

#14 needs a revert and when someone has the time to write a test for the modified version as outlined in #42 then we can commit this again. It does not seem to be an easy affair.

Of course committing the trivial fix in #42 and not demanding an almost impossible to write test is an option...

chx and I are discussing how a test for this might be written. I think we've come up with a solution: checking the HTML for a style.css to see if it reflects the new theme that was selected.

After some discussion with webchick the test is not that impossible after all.

I tested chx's patch and it works, and test passes. I also tried backing out the fix, and the test correctly reports the failure of the code to work.

That makes this RTBC, so I went ahead and Ced it. Thanks!! :)

Reopening... the HTML header is not printed, because template_preprocess_page is called *before* system_preprocess_page. That should do it, and pass the test.

Awesome. Thanks!

I was concentrating too much on the breakage and not what was emitted here. WTF? We never, ever put anything on screen or in source that yells "Drupal" at you. Yes, fingerprinting is possible and easy but actually putting in Drupal in the source code? I dislike that, it does not mesh with us being a framework.

Site information should contain the control for generator IMNSHO.

Exposing a setting for this is completely overkill. Down with settings that only 0.000000001% of sites will ever use. Feel free to make Generator Alterator module in contrib that exposes one, however.

If you strongly dislike it, there is a theme function which can be overridden. Simply return ''; and voila. No meta generator tag.

I don't see the issue.

I can confirm that this is easy to theme. :-)