API page: http://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_g...

Enter a descriptive title (above) relating to drupal_get_token, then describe the problem you have found:

Looks like the comment should be folded in too.

Files: 
CommentFileSizeAuthor
#17 drupal_get_token_return-1787876-17.patch664 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 49,268 pass(es).
[ View ]
#11 1787876-11.patch808 bytescirage
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es).
[ View ]
#3 1787876-3.patch567 bytesBrockBoland
PASSED: [[SimpleTest]]: [MySQL] 41,697 pass(es).
[ View ]

Comments

larowlan’s picture

Issue tags:+#pnx-sprint

Tagging

BrockBoland’s picture

Assigned:Unassigned» BrockBoland

Taking a look at this

BrockBoland’s picture

Assigned:BrockBoland» Unassigned
Status:Active» Needs review
StatusFileSize
new567 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,697 pass(es).
[ View ]

I'm not sure if my explanation is entirely accurate, but I'm pretty sure it meets documentation standards if nothing else.

joachim’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to me.

webchick’s picture

Component:base system» documentation
Assigned:Unassigned» jhodgdon

Since this is about docs, kicking it to Jennifer.

jhodgdon’s picture

Status:Reviewed & tested by the community» Needs work

I am not comfortable with this documentation:
- "always the same" -- only as long as the config variables for salt and private key don't change.
- 43 characters -- that is very specific. How do you know it is always 43 characters, and is it really necessary to say that even if it is always 43 characters?

jhodgdon’s picture

Assigned:jhodgdon» Unassigned
joachim’s picture

> the config variables for salt and private key don't change

Are those really likely to ever be changed on a site?

It's useful to understand that the purpose of this function is to give you something that's reproducible, because that's how the whole token system works: you make a token to give to the user, and then when the user performs an action, you *re-create* the token and compare it with what the user gave you. That's an important idea to grasp, and I'd be wary of watering it down by trying to be ultra-precise.

BrockBoland’s picture

I'm not sure why it's always 43 characters, but gapple mentions it in a comment on the API doc, and I confirmed this by testing it with a bunch of different values.

jhodgdon’s picture

Of course they are not *likely* to change, but as they're just config variables, I don't think I would word documentation to say that they *cannot* change. I think I would word it to say that as long as these configuration options aren't changed, it's unchanging, because that is always the case.

cirage’s picture

Status:Needs work» Needs review
StatusFileSize
new808 bytes
PASSED: [[SimpleTest]]: [MySQL] 47,565 pass(es).
[ View ]

drupal_get_token() calls drupal_hmac_base64() to calculate the hash. By definition the length of a sha-256 hash has a fixed length of 256 bits (32 bytes). Base64 encoding gives a length of 44 bytes including one padding character ("=") which will get removed by the function drupal_hmac_base64().

Therefore the the length of the return value of drupal_get_token() is always 43.

jhodgdon’s picture

Status:Needs review» Needs work

Thanks... but my comments in #6 were not addressed. Also, this patch is not formatted correctly. It needs to be all one paragraph with complete sentences, with each line wrapping as close to 80 characters as possible without going over. I am also not sure that we need to document why the length is 43 characters. What we need to document is how/when the return value is the same or different.

cirage’s picture

Status:Needs work» Needs review

This was my first post to an issue - let's try again ... the reason because returning fixed length token might not be interesting so let's remove this part and keep it short:

"An url safe token (fixed string of 43 chars) which will always be the same for a given $value on the same site"

jhodgdon’s picture

Status:Needs review» Needs work

See #6. "always" is not accurate -- it is only correct as long as certain settings in settings.php do not change.

Also, as far as grammar/wording: it should say "A URL-safe token..." and use "characters" not "chars", and there needs to be a comma before "which".

cirage’s picture

What's about:

"A url safe token (fixed string of 43 characters), which will always be the same for a given $value on the same site, as long as the session_id remains unchanged and $drupal_hash_salt is not modified in settings.php"

jhodgdon’s picture

#115 is still not accurate/complete. Look at the code... How about this:

A 43-character URL-safe token for validation, based on the user session ID, the global $drupal_hash_salt variable from settings.php, and the 'drupal_private_key' configuration variable.

Albert Volkman’s picture

Status:Needs work» Needs review
StatusFileSize
new664 bytes
PASSED: [[SimpleTest]]: [MySQL] 49,268 pass(es).
[ View ]

Sounds good. Here ya go.

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

I'm obviously fine with that patch. :) Any other opinions before we commit it?

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x and 7.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.