PHP Notice: Undefined index in uc_credit.module and uc_store.module

It's really just related to the level of error reporting you have turned on in PHP. They are most likely just representative of older Ubercart code using indexes in arrays that don't exist and expecting either a valid value or a NULL value (like they're getting here). Future versions of our code will be E_ALL and E_STRICT compliant, but these notices don't represent actual bugs in the code.

We've experienced the same error, but it also showed up in the HTML output of the page when at the root url of the site, with empty GET. It seems like the Drupal way of doing this is to use arg(#), but there was a comment in the code about the 'arg' function potentially being unavailable at that scope. This may be true for some cases, but for me it worked by assigning the arg(1) results into a variable, and using that var to do the session closing logic.

/**
 * Implementation of hook_exit().
 */
function uc_credit_exit() {
  $arg1_val = arg(1);
	
  // Make sure sensitive checkout session data doesn't persist on other pages.
  if (isset($_SESSION['sescrd']) && (!isset($arg_val) || $arg_val != 'checkout')) {
    unset($_SESSION['sescrd']);
  }
}

This should probably be fixed. I'm seeing tons of these warnings in apache error_log.

Well, as a matter of procedure, there's nothing here to "be fixed" yet.

The way issue reports work on drupal.org is: you submit an issue, discuss, come up with a bug fix in the form of a patch, post the patch here with status "needs review", wait for someone other than the original poster to verify the patch works and mark the issue as RTBC. Only then should the patch be committed.

This issue is still at the "discuss" phase. Nothing so far indicates this is an Ubercart problem and that the suggested modification fixes the issue. Ubercart has become too big and used by too many people to just go changing lines of code without understanding *why* a problem occurs and without checking for potential side-effects of a "fix". While it MAY BE a problem with Ubercart (I will not rule that out), the fact that the majority of users aren't seeing this warning means the issue isn't as clear cut as you portray it.

In fact, if you search for "Undefined index: q" on drupal.org you will see this is a quite general problem with Drupal, and happens for all sorts of different modules including core under many circumstances. One common thread seems to be running Drupal on IIS, since IIS mis-interprets URLs like http://www.example.com/?q=some/drupal/page when an index page hasn't been set on the server. That URL form is used throughout Drupal and Ubercart, especially in the shipping quote and payment JavaScript, which imay be why you encountered the error message in uc_credit. Another common situation is when clean URLs are turned off but a module tries to reference a clean URL.

What needs to be done at this point is to document why the error occurs, point out what if anything Ubercart is doing wrong that causes the error, and post a patch to address the mistake. Then the patch can be tested by the community to make sure it works and doesn't cause other problems. Even if you're not a developer you can help by providing information, not just saying "me too". For example, what versions of Drupal, Ubercart and PHP are you using? What web server? What are the steps needed to reproduce the error? When does it happen (on what pages, what is the user doing when the error occurs)? I personally don't see this error, so I can't help debugging - it's up to members of the community like you who are having the problem to diagnose what's going on.

Tagging

I get the above errors when browsing with chrome if that helps.

I get the above errors when running the site on a LAMP stack, so it's nothing to do with IIS, or at least not limited to it. It also only happens when loading the front page of the website, where $_GET['q'] isn't set. It's fine on all other pages, where there is a value for $_GET['q']. I'm using apache2, mysql 5 and latest stable versions of Drupal & ubercart.

Perhaps we could check to see if the variable is set before using it?

I can confirm that I too only see this on the homepage.

You're wrong, PHP is wrong. Mr. Ubercart said there's nothing wrong with the code.

I'm not entirely sure who "Mr. Ubercart" is - no one by that nickname has commented on this issue - perhaps you updated the wrong one?

rszrama did say:

They are most likely just representative of older Ubercart code using indexes in arrays that don't exist and expecting either a valid value or a NULL value (like they're getting here). Future versions of our code will be E_ALL and E_STRICT compliant, but these notices don't represent actual bugs in the code.

However this is the latest version of the code, which appears not to be E_ALL compliant. He said all future versions would be and this was older code, but it looks like this instance was missed if that's the case. Therefore in order to be E_ALL compliant as you wish it to be, you should fix this warning.

Don't worry about it, stella. He's poking fun at me. Since my comment, I have stopped maintaining Ubercart and the E_ALL banner has been picked up by others. I think there are other related issues, but I think they're targeting the 3.x branch for E_ALL compliance and not 2.x. TR or Island Usurper would know more.

There are a bunch of outstanding E_ALL issues - note the issue tag of "Ubercart E_ALL" on this thread. To see them all, go to http://drupal.org/project/issues/search/ubercart?issue_tags=Ubercart%20E...

I'd be happy to commit a patch that fixed this, if a patch were offered and tested. I would have liked to use cha0s' patch from last year, but it's too old now to properly apply. I had one person offer to take on this task, but he never followed through. So we're still at the stage I mentioned in the last paragraph of my comment #5 above - someone needs to roll a patch so it can be tested.

Argh! I don't like the sound of that "we won't fix 2.x" because there are URIs where I get 3 pages of notices! That's really annoying.

Anyway, there is your patch.

First of all, if the session is not set, you can just exit immediately. If $_GET['q'] is not set, just unset(). And if the new page is not checkout, also unset().

That's a lot cleaner. You'll have to forward-port to 3.x now... 8-)

Thank you.
Alexis Wilke

P.S. To some of the others, the next version is NOT for Drupal 6.x.

Someone who is experiencing the problem needs to review this and confirm the patch in #14 fixes the situation.

Please don't imply that I in any way said "won't fix in 6.x" - my definitive answer as the branch maintainer is spelled out in #13. If you want E_ALL compliance in 6.x, then someone is going to have to do the work to write the patch, address any concerns raised, and see that the community tests and reviews the patch.

Cool 8-) I'll continue to submit patches then. And create separate issues for anything that cannot be fixed easily.

This one patch does not make any changes to the uc_store as mentioned somewhere. But I did not get those errors and the previous fix seems to point to uc_credit only.

Thank you.
Alexis

#14 prevents the uc_credit warnings for me. Thanks!

They are still showing up for uc_store, but I imagine the fix for that would be similar.

anschauung,

I don't recall having that notice yet.

I would do that if indeed $_GET['q'] may not be defined.

  $q = isset($_GET['q']) ? $_GET['q'] : '';
  $_SESSION['uc_referer_uri'] = $protocol .'://'. $_SERVER['SERVER_NAME'] . $GLOBALS['base_path'] . $q;

Thank you.
Alexis Wilke

subscribing

This is a reroll of #14 to include #18. Only moved the comment for consistency with other uc code.

This logic changes in #14 are sound and this fixes all PHP notices for me.

I just implemented the patch from #20 and the notice on the front page went away, I haven't seen any others yet. Thanks for the patch.

Marking this as RTBC

Thanks. Committed.

I was seeing the same thing in Chrome only and on the front page only. The patch in #20 remedied that for me too.

Cheers!