Auth / login / cookie problems, proposed fix

PS: I tried to confirm / reproduce the sub.domain.com vs. domain.com login problems. I couldn't (tested with FF and IE).

I.e. i only know / can reproduce the embedded vs. standalone login problems. Could anyone describe a step by step guide to reproduce a non-embedded login problem?
If you login in sub.domain.com and then change to a site: domain.com, you have to login again of course. With url rewrite, this is already fixed. But that's about the only issue. Because login works for me in all cases.
Theoretically, you should be able to reproduce the issue by first creating to anonymous sessions / cookies, one with accessing G2 with domain.com, the other by accessing G2 with www.domain.com.
Then try to login in G2 by accessing it with www.domain.com. If there was a problem, it should arise here. But it doesn't.
When accessing G2 by domain.com, the www.domain.com cookie won't match, because its domain .www.domain.com doesn't match domain.com or a substring of domain.com.

edit:
i've searched the forums for login problems since march 1st.
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=28459&highlight=login
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=28069&highlight=login
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=27288&highlight=login
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=27028&highlight=login (2, 3 people complaining)
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=27017&highlight=login (definitely using embedded/standalone)
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=26590&highlight=login
* http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=26625&highlight=login

it could also be session timeout related.

at least 1 of these guys isn't using embedded G2. so yes, there seems to be a problem. but do we know the cause? can we reprodoce it? no.
but the cause for the embedded G2 is known and could be fixed easily with Fix B.

and probably, some of these issues were caused by a bug that was fixed in http://gallery.menalto.com/index.php?name=PNphpBB2&file=viewtopic&t=26590&highlight=login, i.e. they still used an old version.

Thanks for doing the research, valiant!
I tested Moz,FF,IE,Opera on win2k and FF,Safari on OSX.. all behaved as expected: with a / and /path cookie and accessing a url in /path it always listed the /path cookie first in $_SERVER['HTTP_COOKIE']. In all cases I tested setting the / cookie first and setting the /path cookie first (clearing cookies between tests, of course).

So, please proceed with the fix!

btw, see http://bugs.php.net/ for info about filing php bugs (we think it may be a bug that $_COOKIE contains the last listed, ie least specific, cookie when multiple values are sent..)

mindless, I have a fix now.

As discussed:
In GallerySession:
- don't set domain

New method in GalleryUtilities:

    /**
     * Fix the superglobal $_COOKIE to conform with RFC 2965
     *
     * We don't use $_COOKIE[$cookiename], because it doesn't conform to RFC 2965 (the
     * cookie standard), i.e. in $_COOKIE, we don't get the cookie with the most specific path for
     * a given cookie name, we get the cookie with the least specific cookie path.
     * This function does it exactly the other way around, to a) fix our cookie/login problems and
     * to b) conform with the RFC.
     *
     * This function reevaluates the HTTP Cookie header and populates $_COOKIE with the correct
     * cookies.
     *
     * @param boolean force the reevaluation of the HTTP header string Cookie
     * @static
     */
    function fixCookieVars($force = false) {
	static $fixed;
	if (!isset($fixed) || $force) {
	    $fixed = true;
	    if (isset($_SERVER['HTTP_COOKIE']) && !empty($_SERVER['HTTP_COOKIE'])) {
		$allCookies = explode(';', $_SERVER['HTTP_COOKIE']);
		/*
		 * Get rid of less specific cookies if multiple cookies with the same NAME
		 * are present. Do this by going from left/first cookie to right/last cookie.
		 */
		$_COOKIE = array();
		foreach ($allCookies as $cookie) {
		    /* Split NAME [=VALUE], value is optional */
		    $cookie = explode('=', $cookie);
		    $key = preg_replace('|\s|', '', $cookie[0]);
		    $value = isset($cookie[1]) ? $cookie[1] : '';
		    if (!isset($_COOKIE[$key])) {
			$_COOKIE[$key] = $value;
		    }
		}
	    }
	}
    }

This method could be called in bootstrap.inc or in GallerySession::init().

It fixes the embedded/standalone issue and the www. vs. domain.com issue.
Tested with Win XP FF/IE, FF linux x86 Gentoo.

But it has also a disadvantage:
1. login in G2 without url rewrite module in a www.domain.com page as admin
2. activate url rewrite module with base address domain.com
3. logout
4. browse to www.domain.com -> still logged in as admin
5. click on logout. logout link has domain.com style and not www.domain.com -> you can't loggout the www.domain.com cookie/session unless you manually copy the logout link and add a www. at the beginning.

-> should the url rewrite module delete the old cookie when it's activating and create a new one?

But the good news is that you can browse to www.domain.com and login and remain logged in even if url rewrite is installed (the redirect automatically redirects to a baseaddress address and then the cookie is set).

Questions:
1. Where should we move the fixCookieVars() code? IMO, we don't need it in bootstrap.inc, this would slow down the public downLoadItem requests.
It's needed
- before / at the start of GallerySession::init
- in GalleryEmbed::logout because it first gets the cookie data, then the Gallery session.

2. What about the described problem case? Is a change to the url rewrite module necessary (reset cookies).

3. Unit tests: It's a minor change.
- But we could create a unit test that manually populates $_COOKIE with the wrong data and HTTP header Cookie: with the some cookie data and then check the init session call to check if the right sessionId was picked
- can't test the www. vs. domain thing because it's up to the browser what he decides. but we could make sure that G2 doesn't set the cookie domain in a test.

1. is GalleryInitFirstPass a good place to call fixCookieVars?

2. the problem case you describe is a problem of rewrite module.. ideally it should use the hostname in the request and never jump from www. to domain.com. we should proceed with this fix and inform pelle of this issue.

3. yup, those sound like good tests.

do you need a preg_replace call, or is trim($key) sufficient? (maybe trim runs a little faster?)

mindless, thanks for the answers!

@trim: right, should be faster, i.e. not loading the regexp engine.

@1. / GalleryInitFirstPass: yes. GallerySession::init right after global $gallery; would be even better IMO. if you do something with sessions, you have to use gallerysession.class anyway, but not necessarily GalleryInitFirstPass.
And GalleryEmbed::logout needs to call it too.

@IE: thanks. well, if we wanted to go for sure, we had to set the domain explicitely to alway the same value, i.e. .domain.com by default. but as described in this thread, this would have disadvantages. I'd say we stick to the "don't set domain" pattern until we get a user that complains about IE login problems.

Don't want to throw this thread offtrack but I can confirm that the cookies problem in a slightly different way..

For Example (This is a real life example)

mysite.com has WP + G2 installed
testing.mysite.com has WP+G2 installed

G2 has a problem trying to log into testing.mysite.com (You have to use a different browser to get around the issue). Authentication is successful but it seems to be unable to set the cookie thus the login fails.. and you show up as guest again...

I found if the G2 cookie exists for mysite.com then G2 has problems logging into testing.mysite.com

Forcing the www in the mysite.com correctly fixes the issue and then allows the subdomains to login as the mysite.com cookie is not set..

Yeah I believe so also but it was worth just checking and reconfirming. I be able to simulate this issue again should you want to check your solutions..

wow, that was a fast fix for your bug report! (but a pretty slow fix since the bug was first filed in 2002! but closed as "bogus")

valiant, if you haven't done so already, might be worth looking at php's source code to see how they parse it..

time to build a simple test case and file another bug?

ok, i was wrong.
the http cookie header is urlencoded, so it's really easy.
i just have to do some checks on the name/values.

here's the new version of the diff:

http://dev.nei.ch/cookie_fix.diff.gz

this is the php source code which i should mimic now in G2...
I'm ok with alternative #1, not sure about alternative #2.

PHPAPI void php_register_variable_ex(char *var, zval *val, pval *track_vars_array TSRMLS_DC)
{
	char *p = NULL;
	char *ip;		/* index pointer */
	char *index;
	int var_len, index_len;
	zval *gpc_element, **gpc_element_p, *tmp;
	zend_bool is_array;
	HashTable *symtable1=NULL;

	assert(var != NULL);
	
	if (track_vars_array) {
		symtable1 = Z_ARRVAL_P(track_vars_array);
	} else if (PG(register_globals)) {
		symtable1 = EG(active_symbol_table);
	}
	if (!symtable1) {
		/* Nothing to do */
		zval_dtor(val);
		return;
	}

	/*
	 * Prepare variable name
	 */
	ip = strchr(var, '[');
	if (ip) {
		is_array = 1;
		*ip = 0;
	} else {
		is_array = 0;
	}
	/* ignore leading spaces in the variable name */
	while (*var && *var==' ') {
		var++;
	}
	var_len = strlen(var);
	if (var_len==0) { /* empty variable name, or variable name with a space in it */
		zval_dtor(val);
		return;
	}
	/* ensure that we don't have spaces or dots in the variable name (not binary safe) */
	for (p=var; *p; p++) {
		switch(*p) {
			case ' ':
			case '.':
				*p='_';
				break;
		}
	}

	index = var;
	index_len = var_len;

	while (1) {
		if (is_array) {
			char *escaped_index = NULL, *index_s;
			int new_idx_len = 0;

			ip++;
			index_s = ip;
			if (isspace(*ip)) {
				ip++;
			}
			if (*ip==']') {
				index_s = NULL;
			} else {
				ip = strchr(ip, ']');
				if (!ip) {
					/* PHP variables cannot contain '[' in their names, so we replace the character with a '_' */
					*(index_s - 1) = '_';
					
					index_len = var_len = 0;
					if (index) {
						index_len = var_len = strlen(index);
					}
					goto plain_var;
					return;
				}
				*ip = 0;
				new_idx_len = strlen(index_s);	
			}

			if (!index) {
				MAKE_STD_ZVAL(gpc_element);
				array_init(gpc_element);
				zend_hash_next_index_insert(symtable1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p);
			} else {
				if (PG(magic_quotes_gpc) && (index!=var)) {
					/* no need to addslashes() the index if it's the main variable name */
					escaped_index = php_addslashes(index, index_len, &index_len, 0 TSRMLS_CC);
				} else {
					escaped_index = index;
				}
				if (zend_hash_find(symtable1, escaped_index, index_len+1, (void **) &gpc_element_p)==FAILURE
					|| Z_TYPE_PP(gpc_element_p) != IS_ARRAY) {
					MAKE_STD_ZVAL(gpc_element);
					array_init(gpc_element);
					zend_hash_update(symtable1, escaped_index, index_len+1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p);
				}
				if (index!=escaped_index) {
					efree(escaped_index);
				}
			}
			symtable1 = Z_ARRVAL_PP(gpc_element_p);
			/* ip pointed to the '[' character, now obtain the key */
			index = index_s;
			index_len = new_idx_len;

			ip++;
			if (*ip=='[') {
				is_array = 1;
				*ip = 0;
			} else {
				is_array = 0;
			}
		} else {
plain_var:
			MAKE_STD_ZVAL(gpc_element);
			gpc_element->value = val->value;
			Z_TYPE_P(gpc_element) = Z_TYPE_P(val);
			if (!index) {
				zend_hash_next_index_insert(symtable1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p);
			} else {
				if (PG(magic_quotes_gpc) && (index!=var)) {
					char *escaped_index = php_addslashes(index, index_len, &index_len, 0 TSRMLS_CC);
 					/* 
 					 * According to rfc2965, more specific paths are listed above the less specific ones.
 					 * If we encounter a duplicate cookie name, we should skip it, since it is not possible
 					 * to have the same (plain text) cookie name for the same path and we should not overwrite
 					 * more specific cookies with the less specific ones.
 				 	 */
 					if (PG(http_globals)[TRACK_VARS_COOKIE] && symtable1 == Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_COOKIE]) && 
 							zend_hash_find(symtable1, escaped_index, index_len+1, (void **) &tmp) != FAILURE) {
						efree(escaped_index);
 						break;
					}
					zend_hash_update(symtable1, escaped_index, index_len+1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p);
					efree(escaped_index);
				} else if (!PG(http_globals)[TRACK_VARS_COOKIE] || symtable1 != Z_ARRVAL_P(PG(http_globals)[TRACK_VARS_COOKIE]) || zend_hash_find(symtable1, index, index_len+1, (void **) tmp) == FAILURE) {
					zend_hash_update(symtable1, index, index_len+1, &gpc_element, sizeof(zval *), (void **) &gpc_element_p);
				}
			}
			break;
		}
	}
}

mindless, but what if some module devs start creating their own cookies. Or do we strongly discourage that?

I agree that the installer/upgrader and /lib/tools will only be accessed in standalone.
So, if module devs should not use their own cookies, fixing only GALLERYSID would be ok.
Fixing only GALLERYSID is a little easier than fixing all non-array cookies, but not a lot. It's about the same logic. We're talking about 40-50 lines of code difference. Fixing all arrays would take another 50-100 lines

Accepting a list of cookies to fix rather than fixing all non-array cookies could be more complicated than just fixing all of them.

So I went with fixing all non-array, non '[' or ']' containing cookies.

PS: After reading the cookie and http/1.1 rfc, I still think that the http cookie header could actually contain quoted-strings etc. But I won't handle it more complicated than php itself, we wouldn't gain anything.

The hopefully feature-final fix diff:
http://dev.nei.ch/cookie_fix.diff.gz

mindless has pointed out that $_SERVER['HTTP_COOKIE'] could be problematic on IIS. my guess is that it's available for modern ISAPI (quite sure), but not for old ISAPI and not for IIS/php/cgi.

That's a fundamental flaw in our fix. Maybe we need another solution.

I'm showing up late to this party. It looks like you've done a ton of due diligence. Clearly we want to do this the right way and have cookies associated with a given path, but as always we're balked by bugs in the various servers and browsers that we choose to support.

I think that we should aim for simplicity. And simplicity is...

If we start with the simple solution where we use GALLERYSID everywhere with a path of / then this takes care of at least 80% of our users since they'll only have one gallery (and maybe one CMS). This gets us most of the way there, and in fact if you just use one GALLERYSID across multiple galleries it'll work because the same session id will get stored into the two different g2data/sessions dirs. As soon as you log into a G2 it'll change the session id, though so you won't be able to log into both at the same time.

If we want to get THAT working, we could be tricky and say that our desired cookie id is "G2SID . substr(md5($path), 0, 4)" which even with just 4 bytes will give us collision odds of 1:36^4 which I can live with. We'll calculate our expected session id variable that way and always know which sid to look for. Performance hit should be minimal.

Is this the One True Way that we want to develop G2? No. But I think in this case we'll have to be pragmatic and live with it...

Sounds good to me. And put a comment in the code explaining why we're doing this and a note to revisit this decision in, say, a year when the situation may be different.

Sorry, but I still don't get the point how I can fix my recent login problems I am facing since about 1-2 weeks now (I am not able to login at all)? Do I have to install a more recent php5 version or will your patch to the gallery2 sources fix my problems?

bharat, mindless, back to our fixCookieVars + no domain + dynamic path solution:

I have tested Windows XP SP 2 iis 5.1 + php/cgi 4.3.11 and + php-isapi 4.3.11 and in both cases, i get what we need. $_SERVER['HTTP_COOKIE'] is set.

I don't know why http://koivi.com/apache-iis-php-server-array.php says something different (but google indicates that this is a problem).

It could very well be that for certain IIS configs (old iis, old isapi?, config, ...) we don't get the $_SERVER['HTTP_COOKIE'].

How should we procede?
Our fix is finished and seems to work on apache/iis module/isapi/cgi. But there are strong hints that it won't work on some, maybe old iis setups. What we could do: add isset($_SERVER['HTTP_COOKIE']) to the systemcheck step. (But we only get a value there, if the client browser accepts cookies).

We could also go with fix A, which is quite simple and is very robust. On the negative side, we have a cookie name which doesn't win any beauty contest.

mindless and I were discussing the issue in #gallery...
about Fix A:
- sets the domain to .domain.com unless it's a numerical (IPv4/6) host. in this case, the domain is not set (no subdomains -> no multiple cookies).
- The cookiename should be NAME . substr(md5($galleryId),0,4); because g2dataPath could be equal for multiple installs.

we compiled a pro's con's list:
- pro: no HTTP_COOKIE dependence
- pro: robust
- con: hard to identify which cookie is for which site on same domain
- con: small probability of cookie conflict between sites on same domain
- con: other cookies must also adopt this naming scheme or have conflict between sites in same domain, i.e. the cookies should go through a function which adds the substr(md5(galleryId), 0, 4)
- pro: you can take out all the cookie-path stuff to figure out the right path when embedded
- con: when people move their gallery from one folder to another, due to whatever reason, then we get "can't login" posts. but a FAQ entry would do it.

- we could define('SESSION_ID_PARAMETER', 'G2SID' . substr(md5(galleryId), 0, 4)); in config.php then you could identify cookies more easily

- we have getCookieVar($key) which could be modified to check for $key . getconfig(...), we could also create setCookieVar($key, $value, $extraHeader); and then we could make sure in codeaudittest, that the $_COOKIE, and setCookie, and header('Set-Cookie" are not called anywhere else
- or we just inform our module devs that they should add the id to the cookiename and have the id in the config, no special setcookie function needed in this case

pro's and con's of fixCookieVars:
- pro: fixes all non-array cookies, doesn't need further changes
- con: dependency on $_SERVER[HTTP_COOKIE], reason unknown that some have it not set on MS IIS webserver (see google for problems)
- pro: already implemented and unit tests are there
- con: parsing (time, load), although very lightweight

So, bharat, mindless, got any preferences?

Both approaches are ok. I'd like to finish this task and move on.
I'm tempted to choose the fixCookieVars solution until we get some feedback of new auth/login problems.

Ok, another IRC discussion yielded following consent:
- Go with fixCookieVars
- Fall back to Fix A with unique cookie names if further problems are reported due to missing $_SERVER['HTTP_COOKIE'].

Sidenote: bharat and mindless explained me the ad-hoc cookies for hybrid/slider layouts. Javascript -> user-agent -> Javascript cookies. So these cookies shouldn't be of any concern to server side cookie behavior.

A small clearification on how the rewrite module creates URLs. They are built the same way as with the bulk url generator - it uses makeUrl to create the base adress.

This is not possible any more. When you activate the module it saves the path, not the entire URL base (it used to save 'http://www.example.com/gallery2/', now with 0.9.3 it only saves '/gallery2/').

The cookies thing was very annoying to me!. With core 0.9.14 (28 april) it looks like it is solved. At least it works for me! (IIS v5.0/php cgi/IEv6)
Great Job!

Valiant
For your Pleasure...

Test Results
# Module Class Function Success? Time 
1 search  searchutilitiestest  testsanitizesearchcriteria  OK 0.7052 
2 core  utilitiestest  testgetfilenamecomponents  OK 0.6173 
3 core  utilitiestest  testgetfilenamecomponents2  OK 0.5904 
4 core  utilitiestest  testgetfilenamecomponents3  OK 0.6156 
5 core  utilitiestest  testgetfileextension  OK 0.6007 
6 core  utilitiestest  testgetfilebase  OK 0.5920 
7 core  utilitiestest  testmaliciousbrowserinput1  OK 0.6250 
8 core  utilitiestest  testarrayinput  OK 0.5968 
9 core  utilitiestest  testgetformvariables  OK 0.6467 
10 core  utilitiestest  testgetformvariables2  OK 0.6153 
11 core  utilitiestest  testgetformvariables3  OK 0.5951 
12 core  utilitiestest  testgetformvariables4  OK 0.6033 
13 core  utilitiestest  testgetformvariablesnoprefix  OK 0.6021 
14 core  utilitiestest  testgetformvariablesnoprefix2  OK 0.6109 
15 core  utilitiestest  testgetrequestvariables  OK 0.6040 
16 core  utilitiestest  testsanitizeinputvalue1  OK 0.5957 
17 core  utilitiestest  testsanitizeinputvalue2  OK 0.6373 
18 core  utilitiestest  testsanitizeroundtrip  OK 0.6101 
19 core  utilitiestest  testmakemarkers  OK 0.6392 
20 core  utilitiestest  testconvertpathtourl  OK 0.6203 
21 core  utilitiestest  testrequireonce  OK 0.5983 
22 core  utilitiestest  testshrinkdimensionstofit  OK 0.6408 
23 core  utilitiestest  testscaledimensionstofit  OK 0.6158 
24 core  utilitiestest  testisa  OK 0.5957 
25 core  utilitiestest  testisexactlya  OK 0.6428 
26 core  utilitiestest  testhasrequestvariable  OK 0.6094 
27 core  utilitiestest  testhasrequestvariable2  OK 0.6396 
28 core  utilitiestest  testremoverequestvariable  OK 0.6037 
29 core  utilitiestest  testremoverequestvariable2  OK 0.6110 
30 core  utilitiestest  testremoveformvariables  OK 0.6199 
31 core  utilitiestest  testgetremotehostaddress  OK 0.6003 
32 core  utilitiestest  testispathinlist  OK 0.6156 
33 core  utilitiestest  testprocessingpaths  OK 0.6378 
34 core  utilitiestest  testhtmlentitydecode  OK 0.6079 
35 core  utilitiestest  testentitysubstr  OK 0.6134 
36 core  utilitiestest  testcreatealbumtree  OK 0.6216 
37 core  utilitiestest  testgetfile  OK 0.5949 
38 core  utilitiestest  testgethttpdate  OK 0.6139 
39 core  utilitiestest  testiscompatiblewithapi  OK 0.5922 
40 core  utilitiestest  testgetphpinibool  OK 0.6098 
41 core  utilitiestest  testidentifysearchenginecrawlerfromrequest  OK 0.6285 
42 core  utilitiestest  testunicodevaluetoutf8value  OK 0.5897 
43 core  utilitiestest  testunicodeentitiestoutf8  OK 0.6124 
44 core  utilitiestest  testutf8tounicodeentities  OK 0.5916 
45 core  utilitiestest  testutf8substring  OK 0.6339 
46 core  utilitiestest  testgetservervar  OK 0.6118 
47 core  utilitiestest  testgetcookievar  OK 0.5980 
48 core  utilitiestest  testfixcookievars  OK 0.6209 

Summary
29.4942 seconds elapsed

48 tests run

0 failures.

Ok, new issue. We didn't cover all permutations of browsing in {standalone/embedded}, standalone cookie{not set/set for uid x}, embedded cookie{not set / set for uid y}
where x can, but doesn't have to be y.

We forgot about the case:
- standalone cookie = logged out and
- embedded cookie = logged in
Result:
All embedded G2 pages display correctly, but no images show up.
Reason:
The DownloadItem links (all <img src="url"> urls) in embedded are exactly the same as the DownloadItem links in standalone (for performance reasons).
G2 receives the DownloadItem requests along with at least the standalone cookie, perhaps also with the embedded G2 cookie. But the cookie standard defines that the standalone G2 cookie has precedence. So G2 says "this request was from a user which is logged out from G2. If the Item doesn't have sufficient permissions for guests, i'll raise a permission error".

Bottom line: We have to fix this issue.
We've discussed this issue on irc and here are the results...

Fix "fixCookieVars v.2":
Currently we use the "fixCookieVars v.1" fix for cookie / auth problems.
To extend this fix to cover the new problem, we could do the following:
- When generating urls in embedded G2: we'd have to tag all DownloadItem requests in embedded G2 as such, i.e. with an additional url param &g2_embed=1. This is just an additional line in the UrlGenerator, because we already check for this case at the right place.
- When handling the (DownloadItem) request, fixCookieVars() would check if this url param is set and if it is the case, it would select the least specific cookie with the name GALLERYSID.

Consequences:
- The url rewrite module would have to cover this case too, i.e. the additional parameter g2_embed needs to be somewhere in the short urls.
A proposed solution from irc:
example.com/g2/d/{itemId}-{serialNumber}-{embed}/image.jpg
whereas {embed} is 1 for embedded DownloadItem urls and not set or 0 for standalone urls.
Right now, short urls have the format foo.com/g2/d/{itemId}-{serialNumber}/image.jpg
example: foo.com/d/121-1-1/IM01.jpg (new embedded urls) vs. foo.com/d/121-1/IM01.jpg (standalone = as before) or foo.com/d/121-1-0/IM01.jpg (new standalone with embed = false set)
- The majority on irc argued, that {embed} shouldn't be optional and also be set (to "0") in standalone for consistency.

Pro's and Con's:
+ relative easy fix
- short urls get uglier and whether it is an embedded request has not much to do with {itemId}-{serialId}
- fixCookieVars v.1 (current G2) was already more complicated than the other fix alternative without this additional changes. Now it gets even worse.
- the auth / cookie fix would be distributed over GallerySession, GalleryUtilities, GalleryUrlGenerator and RewriteUrlGenerator + .htaccess.
I'd prefer a local fix.

Plus the old Pro's and Con's of this approach:
+ fixes all non-array cookies, doesn't need further changes
- dependency on $_SERVER[HTTP_COOKIE], reason unknown that some have it not set on MS IIS webserver (see google for problems)
+ already implemented and unit tests are there
- parsing (time, load), although very lightweight

Fix A:
No changes are needed for fix A. Here again a short summary for this alternative.
- sets the domain to .domain.com unless it's a numerical (IPv4/6) host. in this case, the domain is not set (no subdomains -> no multiple cookies).
- The cookiename should be NAME . substr(md5($galleryId),0,4); because g2dataPath could be equal for multiple installs.

Pro's and Con's:
+ no HTTP_COOKIE dependence
+ robust
+ simple, fixed locally in GallerySession
+ you can take out all the cookie-path stuff to figure out the right path when embedded (something less to maintain)
- hard to identify which cookie is for which site on same domain
- small probability of cookie conflict between sites on same domain
- other cookies must also adopt this naming scheme or have conflict between sites in same domain, i.e. the cookies should go through a function which adds the substr(md5(galleryId), 0, 4).
+ But currently no other cookies in G2 need this. install/ and upgrade/ are only used in standalone, same for /lib/tools/. The javascript cookies of the slider / hybrid layouts are not relevant for the server side of G2.

We could define('SESSION_ID_PARAMETER', 'G2SID' . substr(md5(galleryId), 0, 4)); in config.php then you could identify cookies more easily.
Alternatively, set GALLERYSID in GallerySession.class at the top, but we'd have to assume that the global $gallery object already exists to retrieve the galleryId.

My conclusion:
Fix A is simpler, we'd have less code to maintain and the only real downside are cookie names, that are hard to identify if you have mutiple G2 installs on the same server.
Fix "fixCookieVars v.2" could be implemented in the same time, but has the disadvantage of slightly uglier urls and an ever increasing complexity of the total fix.
Therefore, I vote for fix A.

is this still and issue with the latest beta2 of g2? I'm having problems login in after upgrading to g2-b2.

System:
WinXP-Pro-Sp2
FF 1.04

Thanks
-JG