if this passes peer review, I will cherry pick/port it to 2.0 and 2.1 if needed.

Hi Jason,

I've just been looking at this now.
The solution you have does stop the error but in turn it prevents the RSS header functionality from being run and in fact masks the actual problem.
I reproduced this as an admin and received the following two errors on that page under master:

Notice: Trying to get property of non-object in /private/var/www/integration/blog/rsslib.php on line 60

and mid page

Can not change context instance properties!

line 4642 of /lib/accesslib.php: call to debugging()
line 317 of /blog/lib.php: call to context->__set()
line 698 of /blog/lib.php: call to blog_get_context_url()
line 824 of /blog/locallib.php: call to blog_get_headers()
line 234 of /blog/index.php: call to blog_listing->print_entries()

I added debugging to get a stack trace on the first issue, by the looks $rsscontext is empty, and judging by a quick glance at the following code, it should never be empty and should instead default to system context.

The second issue already has a stack trace so is easier to debug.
Line line 317 of /blog/lib.php

$context->contextlevel = CONTEXT_SYSTEM;

That is insanely evil code, under no circumstances should the level ever be updated by itself... thats 100% creating an invalid context.
In 2.2 Petr implemented better context handling (from memory 2.2) and now evil hacks like this can be detected and prevented.

I'll leave the rest up to you, I'd consider fixing both of these on one issue (as both prevent the code).
It'd pay to update the test instructions as well to make sure that a valid RSS header for alternative content is added.
Also keep in mind that because the context stuff was implemented in 2.2 your 20, and 21 branches will differ from your 22 and master branches.

Cheers
Sam

Found MDL-30541 and have commented there as well.

Thanks Jason, looks good to me now.
Cheers
Sam

Code cherry picked across to the other branches

Hi Jason,
I've just had a look at this and i've got some small but still pressing points

• we need to think about code readability (clarity over brevity) when we're coding (that and why are we giving the instruction $rsscontext = $rsscontext (when its not empty) )
• also is this a security issue? is that why a fix for 2.0 branch was included? i'm not sure here.

I'll reopen so that we can move towards more readability :-D

cheers,
Apu

Hey Aparup,

I'll make the code easier to understand, removing the ternary operator in favour of a simple if statement.
I provided a patch for 2.0 because everytime I've asked "when do we stop supporting 2.0?" I've been told December. And when I ask "when? December 1st or December 31st?" I've been told "Yes" ... and with such a trivial one-line patch, I thought I'd include it, to allow some one with more authority on the matter to decide.

All repatched now, with greater clarity, Aparup, could you please look at this before I push it any further through the process?

Note: I also cleaned up the variable initialization, to make it cleaner and easier to read.

Just squished the commits for 2.0 and 2.1 to make it easier to integrate

Thanks Jason, thats been integrated now.

I've included it in 2.0 as you're patch applies cleanly and is minor. (and its still December by your argument :-p)

ps: also a missing context really doesn't sound good at all.

Test result: passed Tested in 2.1 and master

Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

Now... disconnect, relax and enjoy the next days, yay!

Closing...ciao