Forum OpenACS Development: ad_conn behind_secure_proxy_p returns "", breaking if {...}

Collapse
1: ad_conn behind_secure_proxy_p returns "", breaking if {...}
Posted by Frank Bergmann on 11/09/20 10:31 PM
https://github.com/openacs/openacs-core/issues/4

Hi!

OpenACS 5.9.1 und 5.10 branches contain an error.
In my current installation [ad_conn behind_secure_proxy_p] returns an empty string, breaking the if-statement in https://github.com/openacs/openacs-core/blob/master/packages/acs-tcl/tcl/utilities-procs.tcl
Line 1924

I also created this as an issue in GitHub.
I've seen two other issues there, but they don't seem to be handled, even though they are pretty nonsense...

Cheers
Frank

Collapse
2: Re: ad_conn behind_secure_proxy_p returns "", breaking if {...} (response to 1)
Posted by Antonio Pisano on 11/10/20 08:38 AM
Dear Frank, I have commented on the github issue.

All the best

Collapse
3: Re: ad_conn behind_secure_proxy_p returns "", breaking if {...} (response to 1)
Posted by Antonio Pisano on 11/10/20 08:43 AM
Regarding the other issues: our current official channel for bug reporting is https://openacs.org/bugtracker/openacs. For informal discussion about issues, we use this forum.

GitHub is now just a mirror of our codebase. This should change with the release after 5.10, but for now we do not commit to any handling of issues posted there.

Collapse
4: Re: ad_conn behind_secure_proxy_p returns "", breaking if {...} (response to 1)
Posted by Gustaf Neumann on 11/10/20 10:00 AM
As Antonio wrote on github, there is no way to get the empty string result with any recent versions of OpenACS. So, probably you have some local modifications.

is the code, where you observing the behavior available on github or on some other public repository?

Collapse
5: Re: ad_conn behind_secure_proxy_p returns "", breaking if {...} (response to 4)
Posted by Gustaf Neumann on 11/10/20 11:31 AM
when going back in history, i see, that this issue was resolved in 2017.

https://github.com/openacs/openacs-core/commit/05c58b86a65e39091565891128ddc76622928014

Collapse
6: ]po[ V5.1 [Re: ad_conn behind_secure_proxy_p returns "", breaking if {...}] (response to 4)
Posted by Frank Bergmann on 11/10/20 12:06 PM
Hi Gustaf,

some local modification

Nope, we've just got a fresh 5.9.1.

I looked into the master and 5.9 branches, but not into the 5.10 branch. I will do this the next time I find something.

Btw., we're currently working on ]po[ V5.1, which is based on OpenACS 5.9.1 with CSP enabled. So we modified some ~300 source files to move onclick(...) and other JavaScript code to [ script type="..." nonce="..." ] tags.

Thanks for providing the infrastructure!

I've already seen some issues in 5.9.1 with files not working with CSP. I'll report them on the OpenACS bug-tracker (https://openacs.org/bugtracker/openacs), unless they have already been fixed in 5.10.

There will be some other issues / ideas / ... coming up from ]po[ V5.1 development, which I'll post in separate threads.

Thanks!
Frank

Collapse
7: Re: ]po[ V5.1 [Re: ad_conn behind_secure_proxy_p returns "", breaking if {...}] (response to 6)
Posted by Gustaf Neumann on 11/10/20 01:49 PM
It is possible to backport this change to 5.9.1, ... but going forward, and giving people a reason to upgrade makes sense.

yes, as note earlier, 5.10 has a huge amount of security improvements. so, unless the mentioned CSP changes are from PO packages, you might save work by heading towards 5.10.

Collapse
8: Re: ]po[ V5.1 [Re: ad_conn behind_secure_proxy_p returns "", breaking if {...}] (response to 7)
Posted by Frank Bergmann on 11/10/20 05:01 PM
Hi Gustaf,

I gave it a short try some weeks ago, and I immediately got some important error. Some compatibility was broken, but I can't remember how what the actual error was. I gave up at that moment and went for 5.9.1.

Is 5.10 allowed to break something?

If it's not: I could offer to setup a joint testing VM where we would install the latest OpenACS version with ]po[. We've got a test script for ]po[, so it's easy to tell if something is broken.

Cheers
Frank

Collapse
9: Re: ]po[ V5.1 [Re: ad_conn behind_secure_proxy_p returns "", breaking if {...}] (response to 8)
Posted by Gustaf Neumann on 11/13/20 10:16 AM
OpenACS 5.10 breaks nothing important (whatever "breaks" means). OpenACS.org runs the newest version of OpenACS 5.10.

When coming from 5.8, it is recommend to upgrade (at least kernel) to 5.9.* before going towards 5.10. It is not necessary, that everything works perfectly on 5.9, but the server should come up after the restart and has to be able to run further upgrade scripts.

OpenACS 5.10 requires e.g. newer versions on support packages (see https://openacs.org/xowiki/openacs-todo).

Collapse
10: Re: ]po[ V5.1 [Re: ad_conn behind_secure_proxy_p returns "", breaking if {...}] (response to 8)
Posted by Malte Sussdorff on 11/15/20 07:05 AM
Hi Frank,

I can provide you with a project-open 5.0.3 container running of openacs 5.10

It has errors, so you will need to manually go through each of the packages and figure out (systematically) where PO Is still using code that should not be there. (And I did not do it. Just remembered that task from upgrading to OpenACS 4.8)

When I looked at that container some months ago, I think the biggest challenge was the database upgrade from 9.2. to 10.x and higher. That was, if I remember correctly, due to a change in tsearch. Which OpenACS takes care of but PO has its own thing for full text search so I think something there did not work out.

I‘d love to help, but given other engagements (encouraging older developers to adhere to coding standards which have been around since XP, not to mention automated code generation) as well as our transition to OpenAPI based coding (lets say we learned a lot and the amount of technical debt shows it 😊), I can*t

Let me know though if I should build you a pipeline for oacs-5-10 based container. This checks out oaks-5-10 based of Naviserver and loads PO packages into it in a second stage. I think it is already on dockerhub at hub.docker.com/sussdorff/projop:oacs-5-10, but I might be wrong.

I would highly recommend by the way to NOT fork the OpenACS packages, but just use the standard ones and only if there is a very good reason and unwillingness of the community to take the patch and no other way to make the functionality work, to for that single package. I know, painful, but you get used to it (at least I got with PO 5.0 after going the fork way with PO 4.0).