• Do not register here on develop.twiki.org, login with your twiki.org account.
• Use View topic Item7848 for generic doc work for TWiki-6.1.1. Use View topic Item7851 for doc work on extensions that are not part of a release. More... Close
• Anything you create or change in standard webs (Main, TWiki, Sandbox etc) will be automatically reverted on every SVN update.
Does this site look broken?. Use the LitterTray web for test cases.

This happended on TWiki.org, running 4.0.2 with security patches:

  • A user (TWiki:Main.JayNealson) attached many very large images (up t 1GB) to his home page and another topic in the Main web.
  • TWiki:Main.TWikiPreferences has this:
    * Set ATTACHFILESIZELIMIT = 50
  • TWiki:Main.WebPreferences has this:
    * Set FINALPREFERENCES = NOSEARCHALL, ATTACHFILESIZELIMIT, WIKIWEBMASTER, WEBCOPYRIGHT, WEBTOPICLIST, DENYWEBVIEW, ALLOWWEBVIEW, DENYWEBCHANGE, ALLOWWEBCHANGE, DENYWEBRENAME, ALLOWWEBRENAME
  • The user's home page has this meta data set:
    %META:PREFERENCE{name="ATTACHFILESIZELIMIT" title="ATTACHFILESIZELIMIT" type="Set" value="10000"}%
    (I removed it, he restored it to attach more files; I disabled his account)

It looks like TWiki is ignoring the FINALPREFERENCES if set at a lower level via meta data.

-- PTh

I don't think that analysis is correct. See http://twiki.org/cgi-bin/view/Sandbox/Item3252, view it raw, and you will see the meta preference. I have run a unit test that confirms the code is recieiving the correct preference setting.

There must be another reason the upload is permitted. Possibly it is estimating the upload size incorrectly?

I changed the ehadline accordingly.

BTW here is the unit test (paste into PrefsTests)

sub test_Item3252 {
    my $this = shift;
    $this->_setDefaultPref("FINALPREFERENCES", "");
    $this->_setSitePref("ATTACHFILESIZELIMIT", "50");
    $this->_setSitePref("FINALPREFERENCES", "NOSEARCHALL, ATTACHFILESIZELIMIT, WIKIWEBMASTER, WEBCOPYRIGHT, WEBTOPICLIST, DENYWEBVIEW, ALLOWWEBVIEW, DENYWEBCHANGE, ALLOWWEBCHANGE, DENYWEBRENAME, ALLOWWEBRENAME");
    $this->_setWebPref("FINALPREFERENCES", "");
    $this->_setUserPref("FINALPREFERENCES", "");
    $this->_setTopicPref("ATTACHFILESIZELIMIT", "10000");
    my $t = new TWiki( $testUser, $topicquery );
    $this->assert_equals(
        50,
        $t->{prefs}->getPreferencesValue("ATTACHFILESIZELIMIT"),
        $t->{prefs}->getPreferencesValue("ATTACHFILESIZELIMIT"));
    # resave the topic with a %META:PREFERENCE instead
    my $user = $twiki->{users}->findUser('AdminUser');
    $twiki->{store}->saveTopic($user, $testNormalWeb, $testTopic, <<TEXT, undef);
Blah
TEXT
    $t = new TWiki( $testUser, $topicquery );
    $this->assert_equals(
        50,
        $t->{prefs}->getPreferencesValue("ATTACHFILESIZELIMIT"),
        $t->{prefs}->getPreferencesValue("ATTACHFILESIZELIMIT"));
}

CC

I just duplicated the setup you described, and upload of oversized attachments was blocked as expected.

One possibility that comes to mind is the Solaris platform. Is stat getting the file size wrong? it wouldn't be the first time Solaris did something wierd.

I really don't want to start uploading huge files to t.o., so I'm marking this waiting for feedback. Please investigate further by reducing the upload size limit and reproducing the problem. Thx.

CC

Testing on TWiki:Sandbox/TestUpload as TWikiGuest, Sandbox web has 50 KB limit:

  • Main.TWikiPreferences has * Set ATTACHFILESIZELIMIT = 50
  • Sandbox.WebPreferences has * Set ATTACHFILESIZELIMIT = 50
  • Upload 7 KB image image1.png => OK
  • Trying to upload 96 KB image synchro1.png => "Uploaded file is too big. Uploaded file synchro1.png exceeds limit of 50 KB"
  • In topic pref settings (via "More" screen), * Set ATTACHFILESIZELIMIT = 1000
  • Trying again to upload 96 KB image synchro1.png => "Uploaded file is too big. Uploaded file synchro1.png exceeds limit of 50 KB"
  • In Sandbox.WebPreferences, commented out * #Set ATTACHFILESIZELIMIT = 50
  • Trying again to upload 96 KB image synchro1.png => "Uploaded file is too big. Uploaded file synchro1.png exceeds limit of 50 KB"
  • In Main.TWikiGuest, * Set ATTACHFILESIZELIMIT = 500
  • Trying again to upload 96 KB image synchro1.png => OK

-- PTh

I tried on my test system and could not reproduce this.

Can this be a bug in 4.0.2 that has since been fixed?

KJL

Just to confirm again, TestUpload passes on this server.

I don't recall any code changes in the area since 4.0.2, though I suppose it's possible. We are relying on Peter to eliminate the possibility of a broken stat on Solaris, though, as he is the only one in a position to add debugging statements to the code.

CC

OK, after some more debugging:

  • Same behaviour on 4.0.2 and 4.1
  • File cannot be uploaded (as expected), if:
    • Main.TWikiPreferences has ATTACHFILESIZELIMIT = 50
    • Sandbox.WebPreferences has ATTACHFILESIZELIMIT = 50
    • Main.TWikiGuest has ATTACHFILESIZELIMIT = 500
    • This topic has ATTACHFILESIZELIMIT = 1000
  • File can be uploaded (bug), if:
    • Main.TWikiPreferences has ATTACHFILESIZELIMIT = 50
    • Sandbox.WebPreferences has no ATTACHFILESIZELIMIT
    • Main.TWikiGuest has ATTACHFILESIZELIMIT = 500
    • This topic has ATTACHFILESIZELIMIT = 1000

So, the problem is that the Main.TWikiPreferences settings are ignored, e.g. the chain of:

  • TWiki.TWikiPreferences
  • Plugin preferences
  • Main.TWikiPreferences
  • User preferences
  • Current web's WebPreferences
  • Current topic
is not honoured.

-- PTh

I cannot reproduce this error.

I have this

Preference topic Setting Final
TWiki.TWikiPreferences ATTACHFILESIZELIMIT not set No final for ATTACHFILESIZELIMIT
Main.TWikiPreferences Set ATTACHFILESIZELIMIT = 10 No final for ATTACHFILESIZELIMIT
Myweb.WebPreferences ATTACHFILESIZELIMIT not set Set FINALPREFERENCES = ATTACHFILESIZELIMIT
Main.KennethLavrsen Set ATTACHFILESIZELIMIT = 10000 No final
Myweb.AttachmentWorks Set ATTACHFILESIZELIMIT = 10000 No final

And I cannot upload a huge file. I am told the limit is 10 kbytes.

I correct some spellings above. You had written Sandbox.TWikiPreferences. I assume this was a spelling. Otherwise THAT is the reason.

KJL

I can reproduce the error now. Peter is right.

It does not matter if the topic to which you attach has a setting.

But if the user topic has a setting in either meta or normal in topic setting then this overrides the setting in Main.TWikiPreferences when the ATTACHFILESIZELIMIT is made final in Myweb.WebPreferences without being defined there.

KJL

As documented in http://develop.twiki.org/~twiki4/cgi-bin/view/TWiki/TWikiVariables#Preferences_Variables, a user setting is evaluated before a web setting. Setting a preference final in a web doesn't tell us anything, I'm afraid.

The original report had the preference set final at the site level, which ought to be honoured and AFAICT is.

CC

Crawford is right. User settings are evaluated before Web settings.

  • First you set the value in Site preferences.
  • Then you alter the value in user preferences.
  • And finally to make the current value final in web preferences.

It works as described in our docs and also worked in Cairo.

If you define the VALUE AND make it final in web preferences then the user defined value is ignored so that is what admins have to do. You cannot set a default at site level and alter it only in come webs. Either you have ONE site level value that you set final OR you define the value and make it final in all web preferences.

I have confirmed that the user cannot set ATTACHFILESIZELIMIT as a final value in their home topic so as long as the value is defined and made final in the web preferences the protection works.

Changing the order so that user defined settings override web preferences would fix the original problem but it would probably also break something since the current spec is old and has been documented.

I guess the only two things we can do are

  • Learn from this
  • Discard this bug
  • Add a ATTACHFILESIZELIMIT value on all webs on twiki.org.

KJL

How many hours have been wasted with this? At least 3 on my side. All caused by an unfortunate spec change from Cairo to Dakar:

I find the Dakar spec not intuitive. Why has the spec changed?

Things we can learn:

  • Be very very very very careful when changing spec, it can cause lots of wasted time and effort.

-- PTh

Those that changed it must have given it some thought.

Look at TWikiReleaseNotes04x00#Changes_to_the_evaluation_order

It may give some hint of the reason. If someone has the Codev reference with the discussion it could also be a good educational help.

I am not trying to reopen an old discussion or suggesting to change things back. Just trying to understand the good reasons that originally triggered the change. I think I understand partly why now but I am not 100% sure I fully understand the advantages the change gives. I am sure they are in some Codev topic.

KJL

Some info for tracking.

The change of the behavior was done by Crawford in SVN 6292. Documentation was updated but not correctly. The docs were updated in SVN 7508.

Check in text was: Item345: major rewrite of the preferences handling, long overdue. Changed so that the silly overwriting scheme is replaced by hierarchical preferences objects, and resolution is done at lookup time. Contrary to expectations, this is actually faster than the old way. It is also cleaner, smaller, more flexible, and, most importantly, it works

The bug item was Item345 which was a different issue that triggered a complete rewrite including spec changes.

I could not find any Codev refs that discussed this spec change yet and I begin to wonder if the change was in fact a mistake done in the huge rewrite?

When the docs were corrected it happened in connection with fixing something else also. It was Item960. It looks like the doc was simply corrected in general to match the code.

The release note remark says: The rules for preference evaluation (whether the user setting overrides the topic setting etc) have always been a bit confusing and difficult to use. For example, a topic setting would override a user setting, but a user setting would override a web setting, making per-web settings awkward to use. Mainly due to the introduction of hierarchical webs, preferences are now evaluated in the following order

With the old rules the user settings would override a web setting unless it is made final. Isn't that the whole idea of user settings?

I still do not understand this spec change and now that I have tracked the actual change I think it was introduced very silently.

If we want the old behavour back the code change seems to be to swap the two statements in TWiki.pm line 1376

from

    $prefs->pushPreferences(
        $TWiki::cfg{UsersWebName}, $user->wikiName(),
        'USER '.$user->wikiName() );

    $prefs->pushWebPreferences( $this->{webName} );

to

    $prefs->pushWebPreferences( $this->{webName} );

    $prefs->pushPreferences(
        $TWiki::cfg{UsersWebName}, $user->wikiName(),
        'USER '.$user->wikiName() );

KJL

This was in the release notes for a long time before the TWiki-4 release.

http://twiki.org/cgi-bin/view/TWiki04/TWikiReleaseNotes04x00x00#Changes_to_the_evaluation_order

I also described it in http://twiki.org/cgi-bin/view/Codev/DakarPreferenceModel, in the hope of getting feedback.

Note that it may be intuitive to you, as experienced TWiki users, that user preferences override web preferences, but it is counter-intuitive to a lot of other people. The original problem that prompted this change was that twiki application developers were continually surprised when a user setting masked a web setting. Further, I had several mystified users trying to understand why a user setting could override a web setting, but not a topic setting. Especially in a context of hierarchical webs, this just doesn't make sense to most people.

Setting preferences final is not intuitive to an inexperienced twiki user. It's very much a "programmer concept" and not natural to most people.

Despite the documentation, I never received any feedback on this change (other than passive acceptance), so I probably assumed that everyone had RTFM and understood the point. You are welcome to re-open the debate on Codev if you think it is appropriate to do so.

CC

Thanks for this good feedback Crawford.

The TWiki:Codev.DakarPreferenceModel is a topic originally posted by Meredith but with Crawford as contributor. A bit mysterious. The original version of TWiki:Codev.DakarPreferenceModel is from 21 May 2006 ie 4 months after Dakar was released or 7-8 months after the code was changed. On the other hand the text in the doc does not looks like Meredith's language. Meredith raised the topic to debate the complexity. I guess the community was in a "debate overload mode" at that time.

I do understand the arguments for the spec change and they are valid. But I would also assume that some still find the new spec confusing. If it is strange that user settings was overriding web settings then why is it OK that they override site settings? And if the user defines a setting as FINAL ??? then who wins over who?

When I look at my users on the Motion site, 0 users has ever defined a user setting. I still need to check my Motorola users but I bet the picture will be the same. I think it is very rare that users try to override settings except hackers. And it is only when an admin defines something site wide and makes it FINAL in web preferences without defining it also there that an issue occurs. So I am not trying to push for yet another spec change for 4.1 which is also why I keep this report discarded.

But I still want to make sure the community gets a chance to think about it so that WHEN we next time make a major change to TWiki both with access rights and preferences - then we have already discussed the spec and have that settled.

TWiki:Codev.DakarPreferenceModel is a good place to continue that discussion.

For the record - Crawford did describe the spec changes - highlighting that there was a spec change!! - in the SVN 6451 checkin of the release notes. At that time called DakarReleaseNotes. No one reacted on the twiki-dev mailing list.

So a final analysis to LEARN from. This spec change history

  • Codev topic from before 11 Sep 2005 not yet found that discuss the spec change.
  • Coded in SVN 6292 11 Sep 2005 / Bug Item 345.
    • The bug item had very little to do with the spec change
    • The checked in code addressed a handfull of issues.
  • Change was documented in release note SVN 6451 19 Sep 2006 / Bug Item 417.
    • Bug item and doc update was the first checkin of the release notes and contained quite a lot of information.
  • Official User Documentation was updated in SVN 7508 17 Nov 2005 / Bugs Item 960 but again the change was part of a change not related to preference order.

The lessons learned

  • The lack of feedback can probably be explained with the fact the all the steps of the change was bundled with other changes so people simply did not notice.
  • The TWiki project did not have a defined release process or feature/spec change process so noone violated any policy.
  • There was indeed a good reason for the spec change - it was not a bug.
  • It shows what we have already learned that it is not a good idea to bundle many changes to same Bugs item. One bug item per change to give VISIBILITY and TRACEABILITY. And Crawford has been a role model in doing this so it is ironic that this example is with Crawford wink Sorry about that CC wink
  • The 4.1 release process has actually ensured that we now have an efficient and reasonably fast process to get spec changes made visible, discussed, and agreed - either by concensus or vote - and in a reasonable time.

Let us end this here and continue on TWiki:Codev.DakarPreferenceModel which I will update to reflect the current spec.

KJL

ItemTemplate
Summary Upload of outsize files permitted
ReportedBy TWiki:Main.PeterThoeny
Codebase 4.0.2
SVN Range TWiki-4.1, Tue, 28 Nov 2006, build 12081
AppliesTo Engine
Component

Priority Urgent
CurrentState No Action Required
WaitingFor

Checkins

TargetRelease n/a
Edit | Attach | Watch | Print version | History: r17 < r16 < r15 < r14 < r13 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r17 - 2006-12-17 - KennethLavrsen
 
This site is powered by the TWiki collaboration platform Powered by PerlCopyright © 2008-2019 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.
Ideas, requests, problems regarding TWiki? Send feedback