This bug cropped up when using advpoll.module in conjunction with votingapi.module, but since the patch was placed in votingapi.module i'm contributing this issue here.
Advpoll.module is setup to allow anon users to vote, but the following code was needed in votingapi in order for this to really work:
votingapi.module:votingapi_set_vote()


    $result = db_query("SELECT * FROM {votingapi_vote} WHERE content_type='%s' AND content_id=%d AND tag='%s' AND value_type='%s' AND uid=%d AND hostname='%s'", $content_type, $content_id, $vote->tag, $vote->value_type, $uid, $_SERVER['REMOTE_ADDR']);
    while ($vobj = db_fetch_object($result)) {
      votingapi_change_vote($vobj, $vote->value);
      $exists = TRUE;
    }
    if (!$exists) {
      votingapi_add_vote($content_type, $content_id, $vote->value, $vote->value_type, $vote->tag, $uid, $_SERVER['REMOTE_ADDR'] );
    }
  }

where $_SERVER['REMOTE_ADDR'] needs to be added to the sql query and the call to votingapi_add_vote(). It follows that votingapi.module:votingapi_add_vote() needs to modified as follows:


function votingapi_add_vote($content_type, $content_id, $value, $value_type = VOTINGAPI_VALUE_DEFAULT_TYPE, $tag = VOTINGAPI_VALUE_DEFAULT_TAG, $uid = NULL, $hostname=NULL) {
....
....
....
  if (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
    $vobj->hostname = $_SERVER['HTTP_X_FORWARDED_FOR'];
  }
  else {
    $vobj->hostname = $_SERVER['REMOTE_ADDR'];
  }
    if($hostname != NULL)
    $vobj->hostname = $hostname;

I haven't yet tested how this mod will affect other modules that also rely on votingapi. At the very least I wanted to raise this issue. Thanks in advance.

Comments

calebtr’s picture

I'm using your patch and noticed a bug.

Your patch matches against $_SERVER['REMOTE_ADDR'] for votingapi_set_vote() but votingapi_add_vote prefers $_SERVER['HTTP_X_FORWARDED_FOR'].

For anonymous users with HTTP_X_FORWARDED_FOR set, voting_api_set_vote will never match, so the anonymous user will always get poll form and never the results.

Instead, try:

  if (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
    $hostname = $_SERVER['HTTP_X_FORWARDED_FOR'];
  }
  else {
    $hostname = $_SERVER['REMOTE_ADDR'];
  }

    $result = db_query("SELECT * FROM {votingapi_vote} WHERE content_type='%s' AND content_id=%d AND tag='%s' AND value_type='%s' AND uid=%d AND hostname='%s'", $content_type, $content_id, $vote->tag, $vote->value_type, $uid, $hostname);
    while ($vobj = db_fetch_object($result)) {
      votingapi_change_vote($vobj, $vote->value);
      $exists = TRUE;
    }
    if (!$exists) {
      votingapi_add_vote($content_type, $content_id, $vote->value, $vote->value_type, $vote->tag, $uid, $hostname );
    }
  }

calebtr’s picture

Ooops, it's advpoll_load() that relies on the REMOTE_ADDR ... I added in the HTTP_X_FORWARDED_FOR check in both places for safe keeping.

Or possibly, could the problem is that votingapi_add_vote() is not getting passed a $hostname for your patch?

I'm making a mess, so I'm going to cross my fingers and see if this works before filing another issue.

ChrisKennedy’s picture

Version: 4.7.x-1.x-dev » master

Well, the way VotingAPI handles HTTP_X_FORWARDED_FOR is not ideal. It shouldn't choose HTTP_X_FORWARDED_FOR or REMOTE_ADDR.

Instead, it should concatenate HTTP_X_FORWARDED_FOR (if it exists) to REMOTE_ADDR, and then insert the result into the database. This will allow the same HTTP_X_FORWARDED_FOR to be used for different REMOTE_ADDRs.

Another important benefit is that it improves security by allowing the administrator to determine if one REMOTE_ADDR is spoofing their HTTP_X_FORWARDED_FOR in order to vote multiple times.

ChrisKennedy’s picture

And I noticed that I submitted a patch for this two months ago at http://drupal.org/node/108592 ....

JamieR’s picture

StatusFileSize
new2.48 KB

I think this is all the proposed changes rolled into one patch file... so far it seems to be working for me... only time will tell for sure. Thanks!

The patch is against:
/* $Id: votingapi.module,v 1.38 2006/07/01 14:07:46 eaton Exp $ */

JamieR’s picture

Status: Active » Needs review
Wouter Van den Bosch’s picture

Just a quick shout to say that I experienced the same problem with the advpoll + votingapi combo, implemented this solution and found that it works just fine for me now.

Big thanks. Saved me. (Was about an e-government platform having a poll out that's announced in the local media tomorrow, so it kinda had to be working by then.)

davegan’s picture

StatusFileSize
new1.35 KB

I've added a patch - this will work against the current version.

I've changed the code somewhat - registered and anonymous users are checked separately, as registered user should only be able to vote from one location and therefore the IP shouldn't be checked.

Any developers live on this project? This bug is causing big problems for advpoll module, and fivestar had to write extra code into their own module, which shouldn't have been necessary. It would be nice to get this patch tested and rolled out so these problems don't happen any longer.

gabash’s picture

Thank you for the patch davegan, unfortunately this is only half way.

I am looking for a way to enhance the "VotingAPI percent vote user" filter on views so both "Currently logged in User" and anonymous users (within the interval) could get content they have not seen before.

pcs305’s picture

after applying the patch things seemed to be working,

but if anonymous user refreshes the page, the results disappear and the options for the poll is shown.

Then If anonymous tries to vote again the "You have already voted in this poll." message shows.

CANCEL! Caching was enabled!!

Thanks

markus_petrux’s picture

Just a note on accessing the $_SERVER array directly to get the IP. Please note that HTTP_X_FORWARDED_FOR could be spoofed easilly.

If this patch is for D6, then ip_address() in bootstrap.inc should be used. If this is for D5, then I would suggest to simply get the IP from $_SERVER['REMOTE_ADDR'], which is what D5 core does. I'm running behind accel proxies and I set the correct IP into $_SERVER['REMOTE_ADDR'] from my settings.php file, just before anything in Drupal can see that array.

Cheers

poggle’s picture

#8
davegan - April 24, 2008

This patch doesn't seem to work with votingapi 5-1.6
(1 out of 1 hunk FAILED).

Is there an update, or any movement on fixing votingapi?

jbrauer’s picture

Version: master » 5.x-1.6
Status: Needs review » Reviewed & tested by the community

Applied the patch to the latest version of the 5.x branch Voting API and indeed it did fix the issue with Advanced Poll/IE7.

poggle’s picture

Did you apply "votingapi.module_2.patch" against votingapi.module (5.x.1.6)?

I'm still getting an error.

jbrauer’s picture

Yes used the patch from #8 against 5.x-1.6.

poggle’s picture

That's totally bizarre - I'm still getting:

Patching file votingapi.module using Plan A...
Hunk #1 failed at 92.
1 out of 1 hunks failed--saving rejects to votingapi.module.rej

I've tried just about every patch command line, and even tried patching it on a different nix box. Very odd - looks like I may have to resort to (shudder) making the changes by hand in vi :-(

bendybus’s picture

I have drupal 5x with Voting API.

Applied votingapi.module.patch. Anonymous users still can't vote.

Do I need to reset the cache: http://drupal.org/node/193995 ? If so how do you do this: "use PHP to call the votingapi_recalculate_results() function, passing in the content type (node, in most cases) and the content id (the node id, also in most cases)."

schnippy’s picture

Just launched a new voting campaign using advanced poll (5.x-1.0-beta6) and voting api (5.x-1.6). At the last minute, the client decided they wanted to allow anonymous voting as well so we turned that on as well in access control. Checking in a few hours later, we were horrified to note (as in this ticket here) that anonymous votes were overwriting each other. We took down the poll and applied the patch above (davegan, April 24-2008) and verified it was working before sending it back out.

So thanks much for this patch -- any reason why this hasn't made it into newer versions of the API?

eaton’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

schnippy, at present the VotingAPI 1.x branch is only getting bug fixes -- it's possible to cast anonymous votes but oyu have to do the heavy lifting of inserting each vote record yourself, so that it doesn't clear out older ones.

Better anon handling is part of VotingAPI 2.x. If you would like to implement it in a Drupal 5.x module, I'd suggest looking at the vote code from Fivestar for D5 -- it uses the technique that was made an official part of the API in 2.x.