BOINC Drupal Release

RPC regressions causing Android client to not attach to project

Details

Description

Two server-side regression occurred as part of the new ‘consent to Terms of use’ code merges.

  1. create_account RPC broke due to a bug introduced by me: authenticator not being returned for existing accounts.
  2. get_project_config.php not returning master_url.

I believe this regression was introduced when Oliver, at my request, updated the BOINC upstream HTML/ code as part of the merging. As part of PR 1731 (Bootstrap) , David A. added this commit, which made $master_url a global variable. In get_project_config.php, it uses require_once("../inc/util.inc") to load the global variable.

This construction does not work for us in Drupal. This is probably due to how we are loading get_project_config: include_boinc('user/get_project_config.php')

We use PHP include() to load the get_project_config.php file. This in turn runs require_once() on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does not run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result the get_project_config RPC for einstein (and albert but see below) do not return the master_url which is needed by the Android client.

re Albert: albertathome.org/get_project_config.php uses drupal codebase, so there is no master_url. But albert.phys.uwm.edu/get_project_config.php uses the BOINC web code, thus there is a master_url.

re Android client: The 'real solution' is to fix the code in the client to use the correct variable, web_rpc_url_base. However, our code should still return the master_url.

Activity

Ascending order - Click to sort in descending order
Shawn Kwang made changes -
Field Original Value New Value
Summary RPC regression causing Andorid client to not attach to project RPC regressions causing Android client to not attach to project
Shawn Kwang made changes -
Description Two server-side regression occurred as part of the new ‘consent to Terms of use’ code merges.

# create_account RPC broke due to a bug introduced by me: authenticator not being returned for existing accounts.

This has been fixed [PR 2567|https://github.com/BOINC/boinc/pull/2567].

# get_project_config.php not returning master_url.

I believe this regression was introduced when Oliver, at my request, updated the BOINC upstream HTML/ code as part of the merging. As part of [PR 1731 (Bootstrap) |https://github.com/BOINC/boinc/pull/1731] , David A. added [this commit|https://github.com/BOINC/boinc/commit/5ea948224ea15ea53bd8167c78e331951d9e1ece], which made $master_url a global variable. In get_project_config.php, it uses `require_once(“../inc/util.inc”)` to load the global variable.

This construction does not work for us in Drupal. This is probably due to how we are loading get_project_config: `include_boinc('user/get_project_config.php')`

We use PHP `include()` to load the get_project_config.php file. This in turn runs ‘require_once()’ on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does *not* run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result the get_project_config RPC for einstein (and albert but see below) do not return the master_url which is needed by the Android client.

re Albert: albertathome.org/get_project_config.php uses drupal codebase, so there is no master_url. But albert.phys.uwm.edu/get_project_config.php uses the BOINC web code, thus there *is* a master_url.

re Android client: The 'real solution' is to fix the code in the client to use the correct variable, web_rpc_url_base. However, our code should still return the master_url. 		Two server-side regression occurred as part of the new ‘consent to Terms of use’ code merges.

# create_account RPC broke due to a bug introduced by me: authenticator not being returned for existing accounts.

This has been fixed [PR 2567|https://github.com/BOINC/boinc/pull/2567].

# get_project_config.php not returning master_url.

I believe this regression was introduced when Oliver, at my request, updated the BOINC upstream HTML/ code as part of the merging. As part of [PR 1731 (Bootstrap) |https://github.com/BOINC/boinc/pull/1731] , David A. added [this commit|https://github.com/BOINC/boinc/commit/5ea948224ea15ea53bd8167c78e331951d9e1ece], which made $master_url a global variable. In get_project_config.php, it uses {{require_once("../inc/util.inc")}} to load the global variable.

This construction does not work for us in Drupal. This is probably due to how we are loading get_project_config: {{include_boinc('user/get_project_config.php')}}

We use PHP {{include()}} to load the get_project_config.php file. This in turn runs {{require_once()}} on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does *not* run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result the get_project_config RPC for einstein (and albert but see below) do not return the master_url which is needed by the Android client.

re Albert: albertathome.org/get_project_config.php uses drupal codebase, so there is no master_url. But albert.phys.uwm.edu/get_project_config.php uses the BOINC web code, thus there *is* a master_url.

re Android client: The 'real solution' is to fix the code in the client to use the correct variable, web_rpc_url_base. However, our code should still return the master_url.
Hide
Permalink
Shawn Kwang added a comment - - edited

Tristan Olive, can I ask for your PHP help?

We use PHP include() to load the get_project_config.php file. This in turn runs require_once() on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does not run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result there is no master_url variable in our get_project_config.php.

Do you have any insights as to a potential solution?

Links:

Show
Shawn Kwang added a comment - - edited Tristan Olive , can I ask for your PHP help? We use PHP include() to load the get_project_config.php file. This in turn runs require_once() on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does not run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier. As a result there is no master_url variable in our get_project_config.php. Do you have any insights as to a potential solution? Links: Drupal boincwork_get_project_config() includes upstream BOINC get_project_config.php file. BOINC get_project_config uses require_once on the util.inc file. util.inc loads variable master_url
Shawn Kwang made changes -
Description Two server-side regression occurred as part of the new ‘consent to Terms of use’ code merges.

# create_account RPC broke due to a bug introduced by me: authenticator not being returned for existing accounts.

This has been fixed [PR 2567|https://github.com/BOINC/boinc/pull/2567].

# get_project_config.php not returning master_url.

I believe this regression was introduced when Oliver, at my request, updated the BOINC upstream HTML/ code as part of the merging. As part of [PR 1731 (Bootstrap) |https://github.com/BOINC/boinc/pull/1731] , David A. added [this commit|https://github.com/BOINC/boinc/commit/5ea948224ea15ea53bd8167c78e331951d9e1ece], which made $master_url a global variable. In get_project_config.php, it uses {{require_once("../inc/util.inc")}} to load the global variable.

This construction does not work for us in Drupal. This is probably due to how we are loading get_project_config: {{include_boinc('user/get_project_config.php')}}

We use PHP {{include()}} to load the get_project_config.php file. This in turn runs {{require_once()}} on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does *not* run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result the get_project_config RPC for einstein (and albert but see below) do not return the master_url which is needed by the Android client.

re Albert: albertathome.org/get_project_config.php uses drupal codebase, so there is no master_url. But albert.phys.uwm.edu/get_project_config.php uses the BOINC web code, thus there *is* a master_url.

re Android client: The 'real solution' is to fix the code in the client to use the correct variable, web_rpc_url_base. However, our code should still return the master_url. 		Two server-side regression occurred as part of the new ‘consent to Terms of use’ code merges.

# create_account RPC broke due to a bug introduced by me: authenticator not being returned for existing accounts.
** This has been fixed [PR 2567|https://github.com/BOINC/boinc/pull/2567].
# get_project_config.php not returning master_url.

I believe this regression was introduced when Oliver, at my request, updated the BOINC upstream HTML/ code as part of the merging. As part of [PR 1731 (Bootstrap) |https://github.com/BOINC/boinc/pull/1731] , David A. added [this commit|https://github.com/BOINC/boinc/commit/5ea948224ea15ea53bd8167c78e331951d9e1ece], which made $master_url a global variable. In get_project_config.php, it uses {{require_once("../inc/util.inc")}} to load the global variable.

This construction does not work for us in Drupal. This is probably due to how we are loading get_project_config: {{include_boinc('user/get_project_config.php')}}

We use PHP {{include()}} to load the get_project_config.php file. This in turn runs {{require_once()}} on the html/inc/util.inc file, which loads the master_url variable. But somehow in how PHP loads things, the Drupal implementation does *not* run this ‘require_once’ on util.inc. Most likely because it has already been ‘required’ sometime earlier.

As a result the get_project_config RPC for einstein (and albert but see below) do not return the master_url which is needed by the Android client.

re Albert: albertathome.org/get_project_config.php uses drupal codebase, so there is no master_url. But albert.phys.uwm.edu/get_project_config.php uses the BOINC web code, thus there *is* a master_url.

re Android client: The 'real solution' is to fix the code in the client to use the correct variable, web_rpc_url_base. However, our code should still return the master_url.
Hide
Permalink
Tristan Olive added a comment -

I think it may be a scope issue. Try defining the $master_url as global where it is initialized, i.e.:

global $master_url;
$master_url = parse_config(get_config(), "<master_url>");

We had to do this historically for prefs variables so that they were available to Drupal, otherwise they were global variables in the BOINC scope, but not in Drupal.

Show
Tristan Olive added a comment - I think it may be a scope issue. Try defining the $master_url as global where it is initialized , i.e.: global $master_url; $master_url = parse_config(get_config(), "<master_url>"); We had to do this historically for prefs variables so that they were available to Drupal, otherwise they were global variables in the BOINC scope, but not in Drupal.
Tristan Olive made changes -
Comment Request [tristano]
Hide
Permalink
Shawn Kwang added a comment - - edited

Thanks Tristan: I had to add it to two places. I'll create a PR for this for BOINC upstream.

But do you have any idea if this will affect the upstream BOINC web code in an adverse way?

Update: GitHub PR 2580 created should fix this master_url issue for us.

Show
Shawn Kwang added a comment - - edited Thanks Tristan: I had to add it to two places . I'll create a PR for this for BOINC upstream. But do you have any idea if this will affect the upstream BOINC web code in an adverse way? Update: GitHub PR 2580 created should fix this master_url issue for us.
Hide
Permalink
Tristan Olive added a comment -

There should be no adverse effects, but I can only offer the aforementioned commit 9798b829 to support that, too. I can't really find any PHP documentation on globals that says you need to use the global keyword to declare a global variable when it will be used by an including script... and I think that isn't even true normally, so there is just something different about what Drupal is doing.

Show
Tristan Olive added a comment - There should be no adverse effects, but I can only offer the aforementioned commit 9798b829 to support that, too. I can't really find any PHP documentation on globals that says you need to use the global keyword to declare a global variable when it will be used by an including script... and I think that isn't even true normally, so there is just something different about what Drupal is doing.
Hide
Permalink
Shawn Kwang added a comment -

Update: GitHub PR 2580 has been merged.

Deployment: This requires updating the upstream BOINC Web code on our systems.

Show
Shawn Kwang added a comment - Update: GitHub PR 2580 has been merged. Deployment: This requires updating the upstream BOINC Web code on our systems.
Shawn Kwang made changes -
Assignee Shawn Kwang [ skwang ] Oliver Bock [ oliverb ]
Hide
Permalink
Oliver Behnke added a comment - - edited

Changes deployed both sites. Will update the BOINC web code on Monday.

Show
Oliver Behnke added a comment - - edited Changes deployed both sites. Will update the BOINC web code on Monday.
Hide
Permalink
Oliver Behnke added a comment -

Web code updated on both sites and <master_url> now being returned as expected. Will let our users know...

Show
Oliver Behnke added a comment - Web code updated on both sites and <master_url> now being returned as expected. Will let our users know...
Hide
Permalink
Shawn Kwang added a comment -

Have we received any field reports about whether or not Android clients may now connect for the first time?

Show
Shawn Kwang added a comment - Have we received any field reports about whether or not Android clients may now connect for the first time?
Hide
Permalink
Oliver Behnke added a comment - - edited

Not in the forum threads [1][2] I know of. I'll have a look again...

Show
Oliver Behnke added a comment - - edited Not in the forum threads [ 1 ][ 2 ] I know of. I'll have a look again...
Hide
Permalink
Shawn Kwang added a comment -

User reports they can connect a new Android BOINC client to our project.

Show
Shawn Kwang added a comment - User reports they can connect a new Android BOINC client to our project.
Shawn Kwang made changes -
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]

People

  • Assignee:
    Oliver Behnke
    Reporter:
    Shawn Kwang
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: