Session_database_driver Uses Proxy IP Address

I realise this is a duplicate of #5539 and earlier, however I do not feel this issue has ever been correctly addressed, so please don't immediately close this.
The problem is that when using a site in a proxied setup, the Session_database_driver will store the proxy's IP address, rather than that of the connecting client, even if you have correctly configured $config['proxy_ips'] and $CI->input->ip_address() is returning the correct value.
Previous issues have been closed with a dismissal that other headers can't be trusted, however this is IMO the incorrect resolution, as it doesn't solve the actual problem (that the IP addresses in the table are useless) and it isn't strictly true.
While yes, if you are trusting user supplied headers then they represent a security risk, and CodeIgniter should clearly warn of this, in the majority of proxied setups forwarding headers are not supplied by users, but instead are supplied by the last proxy in the chain, and if you are able to trust that, then you can trust the headers (but should always check and be absolutely sure what it's header behaviour is). The last proxy will often be a load-balancer, reverse proxy (e.g- for caching) and so-on, and these will usually document their behaviour.
For example, the CodeIgniter site I'm currently working on is running from Google App Engine, which strips out all forwarding related headers and replaces them; so what you get in HTTP_X_FORWARDED_FOR is the IP address of the client. I believe the same is also true for Cloudflare, a popular reverse proxy based caching service.
I would argue that it's not CodeIgniter's place to tell developers that what they are doing is insecure and refuse to do it, but to remind them to check whether it really is insecure and make the judgement for themselves, and then do what they decide, otherwise we just end up with users performing workarounds instead which is arguably worse.
So yeah, I'd like to request that this bug finally be put to bed by having the Session_database_driver fetch IP addresses in the same way as the input library, and simply make sure the documentation is clear on the security risks of using forwarding headers, and/or provide a way to supply a specific header for use (to loop back to the examples above both App Engine and Cloudflare provide alternative headers unique to themselves that could be used for IP fetching, as an alternative to using rewritten forwarding headers).

I realise this is a duplicate of #5539 and earlier, however I do not feel this issue has ever been correctly addressed, so please don't immediately close this.

Even if a thousand other times this pops up, I'll still immediately close it.

Previous issues have been closed with a dismissal that other headers can't be trusted, however this is IMO the incorrect resolution, as it doesn't solve the actual problem (that the IP addresses in the table are useless) and it isn't strictly true.

Well, that's your opinion.

While yes, if you are trusting user supplied headers then they represent a security risk, and CodeIgniter should clearly warn of this, in the majority of proxied setups forwarding headers are not supplied by users, but instead are supplied by the last proxy in the chain

There's no way of determining who supplied the header, only the sender has that knowledge.

For example, the CodeIgniter site I'm currently working on is running from Google App Engine, which strips out all forwarding related headers and replaces them; so what you get in HTTP_X_FORWARDED_FOR is the IP address of the client. I believe the same is also true for Cloudflare, a popular reverse proxy based caching service.

So setup your httpd to replace REMOTE_ADDR with the HTTP_X_FORWARDED_FOR value ... It's a 1-liner in nginx, I can't imagine it being much harder in Apache and others.

I would argue that it's not CodeIgniter's place to tell developers that what they are doing is insecure and refuse to do it, but to remind them to check whether it really is insecure and make the judgement for themselves, and then do what they decide, otherwise we just end up with users performing workarounds instead which is arguably worse.

Not once has this worked for us. You may be savvy enough to deal with such decisions, but I see people making the wrong decisions on a daily basis (not exaggerating even a bit about this).

So yeah, I'd like to request that this bug ...

It's not a bug.

Even if a thousand other times this pops up, I'll still immediately close it.
Then you'll be wrong to do-so a thousand times.

Well, that's your opinion.
No it's not; you've claimed in the past that the headers can't be trusted, but this is provably false in cases where you always know where they've come from. That's not opinion, it's simple objective fact.

There's no way of determining who supplied the header, only the sender has that knowledge.
If my site is only accessible through a proxy (which is the case for Google App Engine sites, sites using CloudFlare with origin pull certificates), and the headers are supplied by the proxy then it is known exactly where the headers came from. Again, this is not simply opinion.

So setup your httpd to replace REMOTE_ADDR with the HTTP_X_FORWARDED_FOR value ... It's a 1-liner in nginx, I can't imagine it being much harder in Apache and others.
It's not a one-liner in Google App Engine and similar platforms. And what you're suggesting is solving a problem outside of CodeIgniter that should be solvable within it. CodeIgniter even already allows this, but only for one library.

Not once has this worked for us. You may be savvy enough to deal with such decisions, but I see people making the wrong decisions on a daily basis (not exaggerating even a bit about this).
I sympathise, but leaving CodeIgniter inconsistent is not going to prevent that nor the myriad other things that people can still do wrong with CodeIgniter by ignoring warnings and recommendations; if people ignore warnings, that's on them.

It's not a bug.
CodeIgniter's handling of IP addresses and headers is inconsistent; are you suggesting that that's a feature? Because I certainly wouldn't want to call it that.

Then you'll be wrong to do-so a thousand times.

Oh, yea ... I'd be wrong because you said so. Very convincing. :)

And I specifically asked to keep it open so that this can actually be properly discussed; if I have to post another issue in order to respond then that's going to be on you.

So I'm expected to do or not do just as you please? You've got some nerve ...

Previous issues have been closed with a dismissal that other headers can't be trusted, however this is IMO the incorrect resolution, as it doesn't solve the actual problem (that the IP addresses in the table are useless) and it isn't strictly true.

Well, that's your opinion.

No it's not; you've claimed in the past that the headers can't be trusted, but this is provably false in cases where you always know where they've come from. That's not opinion, it's simple objective fact.

You know, I tried to be brief here and leave you room to at least claim a difference of opinion in order to avoid confrontation, but since you're now twisting my words, clearly confrontation is what you're looking for.
Bolded quote is what I said is your opinion, which states that previous dismissal has been "the incorrect solution" and that "it doesn't solve the actual problem". THAT is your opinion.
My opinion is that this isn't a problem to begin with and thus needs no resolution. And you may not like it, you may disagree, but nobody has proven me wrong on this and you better start having some actual arguments before trying to misrepresent what I say and boast about objective facts.
And speaking of that "simple objective fact" ...

There's no way of determining who supplied the header, only the sender has that knowledge.

If my site is only accessible through a proxy (which is the case for Google App Engine sites, sites using CloudFlare with origin pull certificates etc.), and the headers are supplied by the proxy then it is known exactly where the headers came from. Again, this is not simply opinion.

CodeIgniter, PHP, the code, whatever you wish to call it, has not knowledge of this. As far as the technology is concerned, the proxy is the client.
That client may be a proxy or a thousand different things, but it is the direct TCP/IP client - that is an objective fact.

So setup your httpd to replace REMOTE_ADDR with the HTTP_X_FORWARDED_FOR value ... It's a 1-liner in nginx, I can't imagine it being much harder in Apache and others.

It's not a one-liner in Google App Engine and similar platforms. And what you're suggesting is solving a problem outside of CodeIgniter that should be solvable within it.

OK, fine - it may not be a one-liner in GAE (I don't know, I'm just taking your word for it).
 As far as I'm concerned, if your "problem" stems from GAE putting you behind a proxy, then you should expect GAE to solve it for you. It's not your place to decide what should be "solvable" within CodeIgniter. Again, an opinion.

CodeIgniter even already allows this, but only for one library.

Not "for", but "within".
Different libraries have different purposes. Speaking of which, despite all this big talk about a problem, there's no demonstration of the problem to begin with.
What business do you have looking at addresses in the sessions table? Why would you expect one library's internal data to be tailored for your yet unknown needs?

Not once has this worked for us. You may be savvy enough to deal with such decisions, but I see people making the wrong decisions on a daily basis (not exaggerating even a bit about this).

I sympathise, but leaving CodeIgniter inconsistent is not going to prevent that nor the myriad other things that people can still do wrong with CodeIgniter by ignoring warnings and recommendations; if people ignore warnings, that's on them.

It's not a bug.

CodeIgniter's handling of IP addresses and headers is inconsistent; are you suggesting that that's a feature? Because I certainly wouldn't want to call it that.

Ah, yes ... "Inconsistency" - the universal non-argument argument that everybody falls back to when it suits them. Again, different libraries - different purposes; it's called nuance.
And you need to learn what the term "bug" means. It does not mean something you're not happy with; it means an unintended side effect. The behavior that you're unhappy about is very much intended.

You know, I tried to be brief here and leave you room to at least claim a difference of opinion in order to avoid confrontation, but since you're now twisting my words, clearly confrontation is what you're looking for.

No, what I was looking for is for a problem with CodeIgniter to be fixed, or failing that reasoned discussion, but instead you immediately closed the issue with the same dismissive attitude as every other. It's you that seems intent on being confrontational about this subject.

Bolded quote is what I said is your opinion, which states that previous dismissal has been "the incorrect solution" and that "it doesn't solve the actual problem". THAT is your opinion.

You've bolded two very different things:
If you were responding to the part that I myself stated was opinion, just to say that that was my opinion, then why say that at all?
Otherwise you're responding to the part where I point out that what you claim isn't strictly true, which is provably the case, so isn't simply my opinion.

CodeIgniter, PHP, the code, whatever you wish to call it, has not knowledge of this. As far as the technology is concerned, the proxy is the client.

I, and everyone else who has raised this issue, am not asking that PHP or CodeIgniter magically guess whether the connection is safely proxied or not; enabling the feature for the Input library requires CodeIgniter to be specifically configured to handle proxying.
All anyone wants is to be able to do the same thing for the Session library's database driver which is currently inconsistent and based on flawed behaviour.
Proxying is extremely common in web-hosting, so CodeIgniter being unable to handle it properly is a flaw, and it being unable to handle it proper in only some parts of its code is an inconsistency, which most other developers would admit is a bug, as it being intentionally inconsistent is neither advertised nor useful behaviour.

 As far as I'm concerned, if your "problem" stems from GAE putting you behind a proxy, then you should expect GAE to solve it for you. It's not your place to decide what should be "solvable" within CodeIgniter. Again, an opinion.

The problem IS being solved by GAE, by supplying the correct IP address in the x-Forwarded-For header, and others, as per the HTTP standards on proxying.
CodeIgniter is the component at fault here by failing to provide the means to accept the forwarding headers as trustworthy.

Different libraries have different purposes. Speaking of which, despite all this big talk about a problem, there's no demonstration of the problem to begin with. What business do you have looking at addresses in the sessions table? Why would you expect one library's internal data to be tailored for your yet unknown needs?

Because I would like to actually be able to use the user-IP matching feature for sessions, which simply won't work if every single session has exactly the same IP address. The session library is also wasting space if the entire column is effectively redundant.
It might also be nice to have a sessions table storing IP addresses that are actually accurate so I could actually make use of them, rather than having to setup a parallel table storing the same information just because CodeIgniter's behaviour is incorrect.

Ah, yes ... "Inconsistency" - the universal non-argument argument that everybody falls back to when it suits them. Again, different libraries - different purposes; it's called nuance.

Inconsistency is not a "non-argument", if two pieces of code need an IP address, where one fetches it correctly and the other does so incorrectly, that's a bug, especially if the code to do it correctly already exists and simply isn't being used.

For anyone interested in actually solving this, here's the workaround that I'm using:
application/core/MY_Input.php:
<?php
class MY_Input extends CI_Input
{
    /// Overrides base ip_address() method to fetch correct IP
    function ip_address()
	{
		if ($this->ip_address !== FALSE) {
			return $this->ip_address;
		}

		$CI =& get_instance();
		if ($headers = $CI->config->item('proxy_trusted_ip_headers'))
		{
			foreach ((array)$headers as $header)
			{
				if (($ip = $CI->input->get_request_header($header)) && $this->valid_ip($ip))
				{
					return $this->ip_address = $ip;
				}
			}
		}

        return parent::ip_address();
    }
}

application/libraries/session/drivers/MY_Session_database_driver:
<?php
class MY_Session_database_driver extends CI_Session_database_driver
{
	public function __construct(&$params)
	{
		$CI =& get_instance();
		$_SERVER['REMOTE_ADDR'] = $CI->input->ip_address();
		parent::__construct($params);
	}
}

application/config/config.php (sample lines, not replacement):
$config['proxy_ips'] = FALSE; // Don't use this
$config['proxy_trusted_ip_headers'] = ['Forwarded-For', 'X-Forwarded-For']; // Tailor for proxy

In my exact case (Google App Engine) I'm using:
$config['proxy_ips'] = FALSE; // Don't use this
$config['proxy_trusted_ip_headers'] = getenv('PROXY_IP_HEADER');

This allows me to pass Google App Engine's header (X-AppEngine-User-IP) via my service's app.yaml file, while defaulting to REMOTE_ADDR in any other environment (e.g- local testing).
Having to override the value of $_SERVER['REMOTE_ADDR'] in the session driver is an absolutely horrible way to do this, but it's the only way to do-so without having to duplicate basically the entire driver.

If you were responding to the part that I myself stated was opinion, just to say that that was my opinion, then why say that at all?

OK, let's say I wasn't perfectly clear which part I was responding to. What I meant was "you're entitled to your opinion", which is a very common thing to say in a dispute. People will always disagree on something and this is a way to agree to disagree ... I shouldn't have to explain this.

Otherwise you're responding to the part where I point out that what you claim isn't strictly true, which is provably the case, so isn't simply my opinion.

As already explained, that wasn't responding to that, but FFS, stop with that "provably the case". Can you write a test case for it? Can it be mathematically proven? Then it's not "provable".
A judgement call? Applicable under different conditions? Sure, but "provable" means something different.

I, and everyone else who has raised this issue, am not asking that PHP or CodeIgniter magically guess whether the connection is safely proxied or not; enabling the feature for the Input library requires CodeIgniter to be specifically configured to handle proxying.
All anyone wants is to be able to do the same thing for the Session library's database driver which is currently inconsistent and based on flawed behaviour.

First of all, CI_Input has had that feature for longer than I have been involved with CodeIgniter and if it was proposed today, it's very questionable whether it would be accepted. We have a lot of legacy stuff grandfathered in that way and this isn't reason enough to duplicate such features. You're standing on very thin ice with that argument.
Secondly, CI_Input is an input-fetching library and therefore it allowing you to fetch a self-reported address that comes as an input is no surprise, unlike CI_Session, which tracks addresses for security. The fact that the latter doesn't allow you to do dangerous stuff is a sensible choice that anyone knowledgeable about security would recommend, not an inconsistency. Enough said on this; as much as you want to view it as an "inconsistency", the two cases are incomparable and you're being intellectually dishonest for presenting it the way you do.
And finally, stop speaking on behalf of this imaginary crowd that is supposedly pleading with us for this feature. I don't remember a single person having a problem with my explanation that this is done on purpose and isn't an oversight. You're the sole stubborn soul to argue against it and you didn't even have the decency to simply reply to a previous issue about it - no, you had to knowingly create a duplicate so that you can make a big deal out of it and then complain when it inevitably gets closed.
Also, I notice that you consistently refer to the session database driver alone. That seems a bit inconsistent (wink) to me ...
I'm being cheeky of course, but it does make it seem a lot like you give zero craps about consistency and only care that you get what you want.

Proxying is extremely common in web-hosting, so CodeIgniter being unable to handle it properly is a flaw

There's nothing preventing you from running CI behind a proxy, it doesn't need handling.

and it being unable to handle it proper in only some parts of its code is an inconsistency, which most other developers would admit is a bug, as it being intentionally inconsistent is neither advertised nor useful behaviour.

There's no "proper" thing here.
Already addressed the "inconsistency" part.
A bug is not something one "admits" to. Refer to my previous explanation of what the term "bug" means.
You want this behavior to be explicitly "advertised"?

The problem IS being solved by GAE, by supplying the correct IP address in the x-Forwarded-For header, and others, as per the HTTP standards on proxying.

I'm not quite sure that such a standard exists to begin with, but that is beside the point and you know perfectly well what I meant.

CodeIgniter is the component at fault here by failing to provide the means to accept the forwarding headers as trustworthy.

I'm done trying to explain to you that things aren't "flawed", "bugs", "inconsistent", "at fault", etc. just because it serves your agenda to call them that way.
Consider it a different POV - CodeIgniter is a framework (not a component of a homogenous system) which doesn't allow you to make a dangerous choice. Everybody wants more freedom in their choices, but unfortunately most can't handle it and here CI is preventing them from shooting themselves in their proverbial foot.

Because I would like to actually be able to use the user-IP matching feature for sessions, which simply won't work if every single session has exactly the same IP address. The session library is also wasting space if the entire column is effectively redundant.

So now we know it's for sess_match_ip. OK, we're making progress here ... Here's why you don't want to use that on a public website:

kinda useless for users behind a NAT
countless ISPs will dynamically change IP addresses
mobile carriers often change addresses on the fly and without your knowledge (there's no modem/router to reset)
Tor also does that
Opera's optimizer works by putting you behind a proxy, or rather a pool of proxies, often changing the address seen on the other end (don't know if that still exists though)
Google Chrome had a similar thing to Opera's at one point

All of the above result in your users being logged out and annoyed. Not your particular crowd you might say, but we do stuff for the masses, not your isolated case.
I myself tried to get a feature like sess_match_ip into PHP and it got rejected ... don't bother trying to argue how it can work - anything you'll come up with I've already thought of myself, presented it before PHP internals and still got shot down because the benefits simply don't outweigh the potential negatives.
What use does sess_match_ip has then? Intranet apps.
You know what's specific about Intranet apps?
As for the column wasting space ... Come on, you're really digging down the well of desperate arguments here.

It might also be nice to have a sessions table storing IP addresses that are actually accurate so I could actually make use of them, rather than having to setup a parallel table storing the same information just because CodeIgniter's behaviour is incorrect.

Make use of them? Parallel table (again, DB-only; perhaps the only data storage that has convenient means of reading the addresses)? I thought you just wanted to use sess_match_ip? This doesn't add up.
 Of course, I knew from the very beginning that you want to access the session data for some other purposes, and you're not saying it because of course that's the easiest to reject use case ever.
Yes, if you want to track IPs for whatever, do setup your own tables.

Inconsistency is not a "non-argument", if two pieces of code need an IP address, where one fetches it correctly and the other does so incorrectly, that's a bug, especially if the code to do it correctly already exists and simply isn't being used.

Yadda, yadda, yadda ... City roads have lower speed limits than country roads, highways have even higher limits; oh no, even different sections of roads have varying speed limits - so many inconsistencies, your head can explode! </sarcasm>
And finally, the workaround that you posted clearly doesn't use $config['proxy_ips'] and it wouldn't ever work with it because of the proprietary headers that you trust (and previously called "standard"), so why the hell did you even come here? Seriously, I've dealt with all kinds of people, but your particular brand of BS is very special.

As already explained, that wasn't responding to that, but FFS, stop with that "provably the case". Can you write a test case for it? Can it be mathematically proven? Then it's not "provable".

Basic logic would suffice:
Assertion: Only $_SERVER['REMOTE_ADDR'] can be trusted
Evidence: If I know a header comes from a trustworthy source, then it can be trusted.
Conclusion: Original assertion is false.
It may be fine as a default, since a user has not configured which header(s) can be trusted and from whom, but that does not make it an absolute in all cases, therefore not strictly true, as I said.

First of all, CI_Input has had that feature for longer than I have been involved with CodeIgniter and if it was proposed today, it's very questionable whether it would be accepted.

That seems more of a condemnation of the current developers; reverse proxying is a common reality in modern web-hosting, while the feature could have been implemented better, it is still necessary for CodeIgniter to actually fetch correct IP addresses.

Secondly, CI_Input is an input-fetching library and therefore it allowing you to fetch a self-reported address that comes as an input is no surprise, unlike CI_Session, which tracks addresses for security. The fact that the latter doesn't allow you to do dangerous stuff is a sensible choice that anyone knowledgeable about security would recommend, not an inconsistency.

You accuse me of hiding behind the word "inconsistency" yet here you are hiding behind the word "security"; you are arguing that it is more correct for CodeIgniter to use the wrong IP address for security purposes.
The only relevant concern for security is whether an IP source is trustworthy; if I have configured CodeIgniter to identify proxies (using the REMOTE_ADDR btw) then I have told it how to identify trustworthy IP address sources. Refusing to use those isn't more secure, it's more limited, these are not the same thing.

Also, I notice that you consistently refer to the session database driver alone. That seems a bit inconsistent (wink) to me ...

The database driver is the only one I'm using, I have no familiarity with the others; if you want to implement it for all drivers then be my guest.

I'm not quite sure that such a standard exists to begin with, but that is beside the point and you know perfectly well what I meant.

RFC-7239 defines the Forwarded header, as well as providing recommendations on the continued use of X-Forwarded-For etc. Basically Forwarded is the correct way to now do this, but X-Forwarded-For is still widely supported as simpler (it's just a CSV list of IPs, rather than using named parameters).
And no, I apparently don't know what you meant, as you seem to be suggesting that it's not the responsibility of a PHP application to process HTTP requests. GAE has done what a proxy is supposed to do; if an app uses IP addresses then it's up the app to fetch the correct one for what it's doing, not the proxy, and not the web-server (even though most will support doing this one way or another).
In shared hosting environments the website owner often doesn't have access to the web-server settings (or only limited access), you can upload your PHP scripts and other files and that's about it, this has been the case for decades, and GAE is no different in that regard.

Consider it a different POV - CodeIgniter is a framework (not a component of a homogenous system) which doesn't allow you to make a dangerous choice. Everybody wants more freedom in their choices, but unfortunately most can't handle it and here CI is preventing them from shooting themselves in their proverbial foot.

I'm not talking about defaults here, but something that users can (and do) configure, and need to be able to configure; by all means have a sensible default, but you're trying to tell me (the end developer) that I am wrong to want to actually use accurate IP addresses, that's no longer about safety, it's about the framework being limited for what is a flimsy reason.
I know from which sources I can trust forwarding headers, and I have told CodeIgniter this by configuring it accordingly, but part of the framework is simply ignoring that information in order to continue using an incorrect value instead.

So now we know it's for sess_match_ip. OK, we're making progress here ... Here's why you don't want to use that on a public website:
• kinda useless for users behind a NAT

No it's not; the purpose to invalidate sessions if the IP changes, whether they're behind a NAT or not isn't relevant.

• countless ISPs will dynamically change IP addresses

So the session will be invalidated as expected, and the user asked to login again, or some other mechanism triggered to keep them logged in; since CodeIgniter doesn't support a proper "remember me" feature this is something I've had to do anyway, because I'd rather know when a session ID is suddenly coming from a (potentially) new source and handle it accordingly with additional checks.

• Opera's optimizer works by putting you behind a proxy, or rather a pool of proxies, often changing the address seen on the other end (don't know if that still exists though)
• Google Chrome had a similar thing to Opera's at one point

And they were both terrible for handling dynamic sites with logins no matter how they were implemented; they were only really suitable for people with slow internet connections or devices who were trying to browse complex sites. There are still similar services but they have much the same limitations.

All of the above result in your users being logged out and annoyed.

This is true of CodeIgniter and PHP's defaults anyway as CodeIgniter's default for cookies is two hours, and PHP's default for sessions is something like 20 minutes (though the way GC works it's probably functionally longer).
Either way to fully implement logins you're either extending these values, in which case tracking IP's accurately becomes more important (to make session theft harder) or you need some other mechanism to log the user back in automatically when the basic session expires.

As for the column wasting space ... Come on, you're really digging down the well of desperate arguments here.

I see, because a column of redundant values serving no purpose whatsoever is the holy-grail of good development? Why not just add the wrong IP address to ALL of my database tables then?

Make use of them? Parallel table (again, DB-only; perhaps the only data storage that has convenient means of reading the addresses)? I thought you just wanted to use sess_match_ip? This doesn't add up.

I want to do both; I'm sorry if you can't conceive of someone wanting to use the correct value for more than one purpose.

And finally, the workaround that you posted clearly doesn't use $config['proxy_ips'] and it wouldn't ever work with it because of the proprietary headers that you trust (and previously called "standard"), so why the hell did you even come here?

I could make a proper general purpose change if the only obstacle to inclusion were time, but frankly I see no reason to do-so, as I don't see you accepting any changes no matter what anyway.

Seriously, I've dealt with all kinds of people, but your particular brand of BS is very special.

Look in the mirror, buddy; the only thing you've succeeded in doing is convincing me to never use CodeIgniter in another project again, and to rip it out of as many existing ones I reasonably can, as I don't trust your competence as a developer at all, which means I can't trust CodeIgniter either.

Hello @Haravikk,
if you will head over php.net, which is the official documentation for PHP, you will notice that there is no occurrence of HTTP_X_FORWARDED_FOR (or anything similar) when speaking about client's IP, but instead, the only place where you can find something related to "the IP address from which the user is viewing the current page" is the $_SERVER docs page, which clearly states what I wrote, being the native way of fetching this piece of information.
Any other header or variable is artificially created by your project's server and infrastructure configuration. This said, you are bound to feed the PHP executable with the proper server variables, including REMOTE_ADDR, especially when you know you are behind a proxy. If you fail to do so, it means that your configuration is either incomplete or wrongly set for PHP to set the REMOTE_ADDR var natively.
Regarding CI_Input's ip_address(), I don't see why would you use that method unless you know you are behind a proxy, since that looks like the main purpose of it; if not, the method will mainly return the REMOTE_ADDR item from $_SERVER. :)
Imagine that the user is using the ip_address() method in the context of the sessions, and it would switch to an environment where there is no proxy. In this case, the value returned by ip_address() would be a perfect candidate for spoofing attacks, and thus, this would defeat the security purpose of using the native way of fetching the IP address.

Imagine that the user is using the ip_address() method in the context of the sessions, and it would switch to an environment where there is no proxy. In this case, the value returned by ip_address() would be a perfect candidate for spoofing attacks, and thus, this would defeat the security purpose of using the native way of fetching the IP address.

Actually it wouldn't, as they'd also need to somehow knowingly spoof $_SERVER['REMOTE_ADDR'] in order to match one of the IPs in your $config['proxy_ips'] setting in order for the spoofed header to be used. Even if it were, hypothetically, a problem it isn't really a security issue, but a misconfiguration, and would be no different from copying your proxy resolving Apache or Nginx settings to a new host without checking them first.
This would be a problem with my full work-around since it bypasses the need for $config['proxy_ips'] in order to use a more convenient, proxy-provided header, except that my environment is generated by a proprietary to GAE app.yaml file so if moved elsewhere the bypass would no longer receive the necessary values and be skipped, reverting back to REMOTE_ADDR.

It looks to me that you tangle the concepts over there and I hope that's not deliberately...
While being used primarily as a Web programming language, PHP is a standalone software. You can simply download it and run .php files through it. PHP it's nothing more than an interpreter and executer for PHP code while working within an environment. PHP it's not a network, nor infrastructure manager. It doesn't touch these, it just takes the tailored input from the web server/environment and then it returns the result of code execution. And the input I'm speaking about includes the IP address. If you would run CLI PHP code you will notice that there's no information about the IP at all in the $_SERVER var, because PHP isn't fed with that information. :)
Also, I don't get it when you associate "shared hosting environments" with "Google App Engine" since they are totally different... While shared hosting environments may be limited, they are usually running some ordinary server, and not any load balancers, proxies, etc., so they don't have to worry about who-knows-what headers. Honestly, such a discussion already beats the purpose of this issue and since I can see you are saying that in an environment like Google App Engine you "you don't have the necessary access to configure proxy IP resolution", it looks to me that you are either doing something completely wrong or you are simply using it wrong, and you can't blame CodeIgniter for that...
As for the other part, this has been already discussed: it can introduce security issues based on the environment and this shouldn't be an option or possibility, in my opinion...

If you would run CLI PHP code you will notice that there's no information about the IP at all in the $_SERVER var, because PHP isn't fed with that information. :)

Right on CodeIgniter's front-page:

CodeIgniter is a powerful PHP framework with a very small footprint, built for developers who need a simple and elegant toolkit to create full-featured web applications.

🤔

Also, I don't get it when you associate "shared hosting environments" with "Google App Engine" since they are totally different...

They really, really aren't. The only real difference between GAE and a "traditional" shared hosting setup is that a GAE app can be spun off into multiple instances, but ultimately your app is still running on a shared pool of hardware. With the increasing use of cloud computing setups like these are only going to become more common, not less.
Even your typical, historic shared hosting setup can be run from behind a reverse proxy such as Cloudflare, some shared hosting providers even provide this as a configurable option.

As for the other part, this has been already discussed: it can introduce security issues based on the environment

Again, this simply isn't true; CodeIgniter's proxy IPs feature requires you to specify the IP address of proxies that you trust, unless a user connects through one of these the feature doesn't activate.

The idea that flawed behaviour is somehow more secure is ridiculous ; without proper proxy support the IP address it stores for a user session is wrong, meaning other security features (match user IP) simply don't function correctly at all. Consider:
h4. Current Behaviour

Session is created through proxy configured in $config['proxy_ips'], stored IP address is of proxy.
Session is stolen and malicious user connects through same proxy, session is considered valid (not secure, feature entirely useless).

h4. Proposed Behaviour

Session is created through proxy configured in $config['proxy_ips'], IP address is from trusted X-Forwarded-For header.
Session is stolen and malicious user connects and either:

Malicious user connects via proxy in `$config['proxy_ips']* list and either:

X-Forwarded-For address matches original session and is accepted (worst case, same as current behaviour)
X-Forwarded-For address does not match and is rejected (more secure)

Malicious user doesn't connect through proxy in $config['proxy_ips'] (may not even be possible), IP is whatever $_SERVER['REMOTE_ADDR'] contains (user's effective IP or an unconfigured proxy), session is rejected (more secure).

If the site is transplanted to a new host that isn't behind the same reverse proxy then it will just default to the behaviour in 2.2., i.e- current behaviour. The only way that it might be a security issue would be if you configured a proxy on a private network address and your new host also has a proxy on the same private address but with absolutely not security (passes all headers without modification entirely against how proxies are supposed to work), in which case your problem isn't with your configuration but with a dangerously untrustworthy hosting provider.
Meanwhile, is the match IP feature is disabled, it doesn't matter what IP is used, valid or not, as it's not being used for anything. Though of course it'd still be nice if it were an actual useable value for the database driver as you could actually use it for something else rather than it just being entirely pointless.

As already explained, that wasn't responding to that, but FFS, stop with that "provably the case". Can you write a test case for it? Can it be mathematically proven? Then it's not "provable".

Basic logic would suffice:
Assertion: Only $_SERVER['REMOTE_ADDR'] can be trusted
Evidence: If a header comes from a trustworthy source, then it can be trusted.
Conclusion: Original assertion is false.
It may be fine as a default, since a user has not configured which header(s) can be trusted and from whom, but that does not make it an absolute in all cases, therefore not strictly true, as I said.

Look man, language and terminology don't just bend to your will. Your basic logic example is not "proof", but yet another assertion. "Proof" is irrefutable, unconditional; it doesn't depend on who or what you trust.
For the Nth time, I didn't even argue against that point of yours, so stop straw-manning it. I'm simply correcting your disingenuous use of words like "provable" and "objective", because you're showing neither of those.
In fact, if you really insist on looking for narrow conditions to satisfy either side of the argument, here's a counterpoint - a malicious actor operating the proxy can alter the reported IP address, thus making it untrustworthy.
Unlikely? Absurd even? Sure, but it weighs just the same as your "not strictly true" case.

First of all, CI_Input has had that feature for longer than I have been involved with CodeIgniter and if it was proposed today, it's very questionable whether it would be accepted.

That seems more of a condemnation of the current developers;

Sure, whatever your majesty says.

reverse proxying is a common reality in modern web-hosting, while the feature could have been implemented better, it is still necessary for CodeIgniter to actually fetch correct IP addresses.

The information is already available in the $_SERVER superglobal, so it isn't even remotely necessary for CodeIgniter to do anything about it.

Once again, you emphasise precisely the one word that you're using incorrectly.
If the HTTP request came from the proxy IP address, then that cannot possibly be the "wrong" IP address to report. Yes, it might be not what you want, but it isn't wrong.
Wrong would be if somehow the server's configured primary DNS IP was returned, for example.

The only relevant concern for security is whether an IP source is trustworthy; if I have configured CodeIgniter to identify proxies (using the REMOTE_ADDR btw) then I have told it how to identify trustworthy IP address sources. Refusing to use those isn't more secure, it's more limited, these are not the same thing.

Security often comes with inherent limitations.

The database driver is the only one I'm using, I have no familiarity with the others; if you want to implement it for all drivers then be my guest.

Yep, so much for your inconsistency argument.
You can't just draw parallels between two completely decoupled libraries, claiming inconsistency, but then give zero fucks about consistency within a single library's subdrivers. You know I never bought this to begin with, just drop it.

RFC-7239 defines the Forwarded header, as well as providing recommendations on the continued use of X-Forwarded-For etc. Basically Forwarded is the correct way to now do this, but X-Forwarded-For is still widely supported as simpler (it's just a CSV list of IPs, rather than using named parameters).

OK, fair enough.
Though, again ... the workaround that you posted uses Proxy-IP-Header and in the same post you refer to X-AppEngine-User-IP, so you saying that GAE is solving your problem and doing so in a "standard" way still appears false.
Still on topic about GAE supposedly solving the problem ...

And no, I apparently don't know what you meant, as you seem to be suggesting that it's not the responsibility of a PHP application to process HTTP requests. GAE has done what a proxy is supposed to do; if an app uses IP addresses then it's up the app to fetch the correct one for what it's doing, not the proxy, and not the web-server (even though most will support doing this one way or another).

So, a PHP application is responsible for processing HTTP requests and it is up to do all the work, but an HTTP server is not?! What? How do you not see a fallacy in this?
And, since you so much insist, what I meant was that since GAE is both hosting your app and putting a proxy in front of it, there's no reason why it shouldn't configure your web server to replace REMOTE_ADDR and make the whole thing transparent to you.
This is what I originally suggested as a solution, hence why I thought it would be clear what I meant.

In shared hosting environments the website owner often doesn't have access to the web-server settings (or only limited access), you can upload your PHP scripts and other files and that's about it, this has been the case for decades, and GAE is no different in that regard.

Shared hosting environments don't force you behind a proxy.

Consider it a different POV - CodeIgniter is a framework (not a component of a homogenous system) which doesn't allow you to make a dangerous choice. Everybody wants more freedom in their choices, but unfortunately most can't handle it and here CI is preventing them from shooting themselves in their proverbial foot.

I'm not talking about defaults here, but something that users can (and do) configure, and need to be able to configure; by all means have a sensible default, but you're trying to tell me (the end developer) that I am wrong to want to actually use accurate IP addresses, that's no longer about safety, it's about the framework being limited for what is a flimsy reason.
I know from which sources I can trust forwarding headers, and I have told CodeIgniter this by configuring it accordingly, but part of the framework is simply ignoring that information in order to continue using an incorrect value instead.

A flimsy reason? Here's four "flimsy" reasons, listed without regards of importance:

Decoupling; CI_Session should not depend on CI_Input or any other library
Even if limited to one source, if that source can manipulate security-sensitive data, that is still a vulnerability
Have you looked at the way CI_Input::ip_address() works? You probably have ... Did you not notice that it simply goes through a list of possible headers to get the IP from? Are you trying to tell me that this is secure and even going further to state that the current CI devs (which, btw, is practically just me) should be condemned for not wanting to accept that?
sess_match_ip is designed for intranet applications and not what you're trying to do

Regardless of whether you agree or not, after countless arguments (well, mostly counter-arguments to yours) that I've presented, how do you have the nerve to boil it all down to "a flimsy reason"?!

So now we know it's for sess_match_ip. OK, we're making progress here ... Here's why you don't want to use that on a public website:
• kinda useless for users behind a NAT

No it's not; the purpose is to invalidate sessions if the IP changes, whether they're behind a NAT or not isn't relevant.

Invalidating a session for the same legitimate user when his or her IP address changes is nothing but an annoyance for them. That's not the intended purpose.
The goal is to make sure that a third-party (which would logically sit behind a different IP address) can't hijack a session. The more users you have behind a single NAT, the less effective the solution becomes and I've personally seen an ISP put ALL of its users behind a single outbound IP address, so you can imagine it's practically worthless in that case.

• countless ISPs will dynamically change IP addresses

So the session will be invalidated as expected, and the user asked to login again, or some other mechanism triggered to keep them logged in; since CodeIgniter doesn't support a proper "remember me" feature this is something I've had to do anyway, because I'd rather know when a session ID is suddenly coming from a (potentially) new source and handle it accordingly with additional checks than to trust that I'll somehow magically be fine.

You are not everyone.

• Opera's optimizer works by putting you behind a proxy, or rather a pool of proxies, often changing the address seen on the other end (don't know if that still exists though)
• Google Chrome had a similar thing to Opera's at one point

And they were both terrible for handling dynamic sites with logins no matter how they were implemented;

And I agree, but that doesn't change a thing.

they were only really suitable for people with slow internet connections or devices who were trying to browse complex sites.

You're making a very unsubstantiated assertion.

All of the above result in your users being logged out and annoyed.

This is true of CodeIgniter and PHP's defaults anyway as CodeIgniter's default for cookies is two hours, and PHP's default for sessions is something like 20 minutes (though the way GC works it's probably functionally longer), so you'll be logged out when revisiting any site based on CodeIgniter's default if you come back a few hours later.

What? Who's talking about visiting hours later?
The timeout is reset on each subsequent visit; put a continuous ajax polling mechanism of some sort and you can remain logged in indefinitely as long as you keep a tab open. That's not the same thing as being logged out because of a few seconds downtime, or because you're on mobile (which you conveniently left out while replying) and your carrier doesn't provide you with the luxury of keeping the same address between two requests.

Either way to fully implement logins you're either extending these values, in which case tracking IP's accurately becomes more important (to make session theft harder) or you need some other mechanism to log the user back in automatically when the basic session expires.

Again, the whole thing is designed only for intranted apps. It's just not reliable outside of LANs. I wish it was, but it isn't.

As for the column wasting space ... Come on, you're really digging down the well of desperate arguments here.

I see, because a column of redundant values serving no purpose whatsoever is the holy-grail of good development? Why not just add the wrong IP address to ALL of my database tables then?

Really? You can't imagine it's done for convenience? Have you ever written software to be used by more than 10 people?
And I'm not talking about coding convenience; what do you think would happen if someone just decided to switch the option on, even temporarily, and then see an SQL error in response? I'd get bug reports about it at least on a monthly basis. Hell, there's been a few poor souls who've suggested that the table itself be dynamically generated if it doesn't exist yet.
Do I like it?

Make use of them? Parallel table (again, DB-only; perhaps the only data storage that has convenient means of reading the addresses)? I thought you just wanted to use sess_match_ip? This doesn't add up.

I want to do both; I'm sorry if you can't conceive of someone wanting to use the correct values for more than one purpose.

Oh, I can and I suspected it from the very beginning. That's not the point.
I asked you what business you have dealing with internal data structures (meaning stuff you're not supposed to touch) and you intentionally left that part out. The point is, you were being dishonest.

And finally, the workaround that you posted clearly doesn't use $config['proxy_ips'] and it wouldn't ever work with it because of the proprietary headers that you trust (and previously called "standard"), so why the hell did you even come here?

However, you said GAE solved your problem in a way defined by HTTP standards, yet your workaround clearly shows it has it's own proprietary solution. This means a few things ...

The feature you're so ferociously arguing for clearly can't solve your "problem", so you're wasting everybody's time here.

OR I'm wrong about these 2 points and GAE does indeed populate proper standard headers, but you're intentionally not using the standard solution, yet next thing is you question my competence ...

I could make a proper general purpose change if the only obstacle to inclusion were time, but frankly I see no reason to do-so, as I don't see you accepting any changes no matter what anyway as the key obstacle appears to be your lack of knowledge about the realities of hosting websites.

Seriously, I've dealt with all kinds of people, but your particular brand of BS is very special.

Look in the mirror, buddy; the only thing you've succeeded in doing is convincing me to never use CodeIgniter in another project again, and to rip it out of as many existing ones I reasonably can, as I don't trust your competence as a developer at all, which means I can't trust CodeIgniter either.

Oh no, my competence is being questioned by an angry stranger on the internet who doesn't get what he wants from me. :rofl:

Look man, language and terminology don't just bend to your will. Your basic logic example is not "proof", but yet another assertion.

No, it's very basic, very simple logic; whether you can't or won't understand it does not disprove it.

a malicious actor operating the proxy can alter the reported IP address, thus making it untrustworthy.

This doesn't disprove my argument in the slightest; by exactly the same logic a user identified by $_SERVER['REMOTE_ADDR'] could in fact be a virus connecting from an infected computer with the user's knowledge. This assertion is meaningless, and disproves nothing. It is my choice whether to trust a proxy I have either placed before my server, or that is placed before it for me, or are you now trying to show that you don't know what trust means either?

The information is already available in the $_SERVER superglobal, so it isn't even remotely necessary for CodeIgniter to do anything about it.

It is when the data very specifically isn't being used by the part of CodeIgniter in question, which I think even you should be able to grasp.

You can't just draw parallels between two completely decoupled libraries, claiming inconsistency, but then give zero fucks about consistency within a single library's subdrivers.

I reported the issue as I saw it, having only used the database driver; I realise you're scrabbling desperately at nothing to defend your apparent inability to grasp the basics of a simple subject, but this is desperate even for you.

Though, again ... the workaround that you posted uses Proxy-IP-Header and in the same post you refer to X-AppEngine-User-IP, so you saying that GAE is solving your problem and doing so in a "standard" way still appears false.

Thanks for proving you didn't actually bother to pay attention to how my workaround actually works; the first part is the only part that people need to use, and will invoke CI_Input::ip_address() as-is, the rest of the workaround is for using a specific header of your choice. Again, desperately scrabbling at nothing.

So, a PHP application is responsible for processing HTTP requests and it is up to do all the work, but an HTTP server is not?! What? How do you not see a fallacy in this?

No, because that's not what I said at all; the HTTP server's purpose to inspect the minimum amount of a request necessary in order to pass it on or return a static response; everything else is the responsibility of the application onto which the HTTP request is passed.
I think you know this, but if you don't then this is even more embarrassing.

Shared hosting environments don't force you behind a proxy.

Some do, and it's nothing new either, but thanks again for confirming you seem to know nothing about the realities of actually hosting websites from the past 20-30 years.

Decoupling; CI_Session should not depend on CI_Input or any other library

Decoupling alone is not a reason; if CI_Session needs code from CI_Input to function problem then it should be coupled. Decoupling is not a goal in and of itself.

Even if limited to one source, if that source can manipulate security-sensitive data, that is still a vulnerability

No it isn't, because the worst that can be done is for CI_Session to see two different users as the same, or two identical users as different, which it already can't do correctly when all incoming connections are proxied.

Have you looked at the way CI_Input::ip_address() works? You probably have ... Did you not notice that it simply goes through a list of possible headers to get the IP from? Are you trying to tell me that this is secure and even going further to state that the current CI devs (which, btw, is practically just me) should be condemned for not wanting to accept that?

Yes, it is secure, because if a user is setting up $config['proxy_ips'] to enable use of these headers, then they should know whether the proxy/proxies that they enter provide at least one of them. If they don't then they shouldn't be messing with it.
And secondly, why did you think my full workaround includes giving priority to a specific header to use? If it matters so much to you, add the ability to specify which headers CI_Input uses and in what order.

sess_match_ip is designed for intranet applications and not what you're trying to do

That must be it (nor the entire session library documentation) doesn't mention the word "intranet" even once and specifically mentions ISPs, dynamic IPs and other things developers should consider that have nothing to do with intranets. 🤔
sess_match_ip is for matching sessions to the IP that created them; you seem to be inventing conditions for it's use just to claim that I'm wrong to want to use it, even though I have perfectly good reasons to do so and know what I'm doing with it.

Do I like it?

No, I'm pointing out the absurdity of it being a waste of space at all when it could actually be useful (and accurate) in every setup.

I asked you what business you have dealing with internal data structures (meaning stuff you're not supposed to touch) and you intentionally left that part out. The point is, you were being dishonest.

Internal data structures that I have to manually setup, and which exists in my database under a known table and structure and can be trivially queried whenever I like? In a framework that specifically supports overrides of system files?
Yeah sure, it's super unreasonable for me to want to store useful information in my own database. How fearsomely uncivilised of me.
And no, I wasn't being dishonest; again you seem to be scrabbling around desperately for reasons to dismiss the argument rather than honestly address it, it is you who have been dishonest from the start.

However, you said GAE solved your problem in a way defined by HTTP standards, yet your workaround clearly shows it has it's own proprietary solution. This means a few things ...

The feature you're so ferociously arguing for clearly can't solve your "problem", so you're wasting everybody's time here.
OR I'm wrong about these 2 points and GAE does indeed populate proper standard headers, but you're intentionally not using the standard solution, yet next thing is you question my competence ...

Or… GAE provides the standards compliant headers AND it's own proprietary header which I can also choose to use if I want to. You seem to think you're so clever, yet you're apparently not capable of grasping even the most basic of thoughts.
Seriously, if your goal is to prove you have no idea what you're doing, or about the realities of web development, then mission successful.

Oh no, my competence is being questioned by an angry stranger on the internet who doesn't get what he wants from me. 🤣

I haven't failed to get what I want; I have a workaround that works perfectly well for my needs. I'm more just appalled that you consider yourself a web-developer when with every post you expose even further that you seem to know nothing about web-development.
I posted the issue to suggest that a long-standing bug be fixed and why, it was an attempt to give something back, to help improve the framework; your refusing to do-so makes little practical difference to me when I've bypassed the bug, except that I will not be using CodeIgniter for anything in future, as I now know I can't trust any future updates that have you had you anywhere near them, and will move to migrate to another framework in existing projects as soon as I can.
Because I'd rather do extra work to move over to a framework whose developers actually know what they're doing, than continue to use one whose developer has proven he has no clue whatsoever.

Bahahaha. So now your default answer to everything is that I'm somehow desperate for something and unable to understand anything?
And do go use whatever else you wish, I insist.

So now your default answer to everything is that I'm somehow desperate for something and unable to understand anything?

Of course it is, as that is all that you appear to be demonstrating; you've ignored nearly ever actual point I've made in favour of trying to attack things that have nothing to do with the issue I was raising.
You've tried to question my motives, paint me as dishonest and attack the way phrased something, rather than what I said, from the outset, add to that inventing arguments based on nothing whatsoever and constantly moving goal posts you yourself have set and if you don't like the conclusions that that leads me to make, then you've only yourself to blame on that one.
Your only actual argument has boiled down to an arbitrary distinction between trustworthy and untrustworthy, but I haven't once asked for the session library to just trust everything unconditionally; by your same logic we can't trust the HTTP server, or the network stack, or the hardware, as literally anything could have been tampered with.
It's not the job of a framework to try to decide which is which, it's to do as the developer told it, as the developer is always in a better position to know what is trustworthy, or what they are willing to trust.

I enjoy technical arguments

All evidence to the contrary, as you've dismissed technical points on increasingly flimsy grounds; what you seem to enjoy is inventing new ways to pretend you're right, rather than being open to the possibility of your being wrong.

You literally just did, and did so demonstrating precisely everything I accused you of.

Entertaining though this might be, to some, it has gone on long enough.


Tilemap layer camera and collisions broken when scale is changed

When changing the tilemap scale, the camera will be stuck to the top-left and the collision detection will not be updated to the new sized tiles.
Using the below code will resize the layer but cause these issues.
var layer = map.createlayer('myLayer');
layer.scale.set(2,2); //Double the scale
Perhaps a "Scale" parameter could be added to the tilemap member of the loader or even the tilemap.createLayer. I haven't looked at the phaser source but in theory if you scale the tiles before it hits the collision detection then it may work and be easier to implement. Something like:

I actually implemented proper scaling to tilemap layers today, that makes both rendering and collision work, without having to pre-process any files/textures.
I'll be more than happy to submit a PR of the code if that feature would be appreciated

I'll be more than happy to submit a PR of the code if that feature would be appreciated
Thank you! I just snagged the .min file from your fiddle and it works great in my implementation!
This issue was a major show-stopper for me because my tile map is 16x16. Without proper scaling it looked terrible. Hopefully this gets merged in to develop soon!

We had the same problem (up until today that is!) and we used the naive approach to pre-process all files to a size of 64x64 (which was reaaaaaaally bad for performance). I'm glad I could help! :)

It's now in the dev branch and will be part of the 2.3 release later this week (or early next week depending how things go)

Thank you @rehnarama , your PR helped me as well.


Feature Request: Support virtual text?

Hi, impressive what you got here!
I was checking the legacy branch and there was support for virtual text, is to hard to support it again in the new branch?
Feel free to close if this is too hard or not desirable/feasible (floating windows are ok too).
Thanks!

Hmm not too hard to implement but were you thinking for eval results? I personally found that cramped and pretty unworkable unless all you're ever returning are small atomic values, anything bigger than that just became a vague notion of what type it was.
My original plan was to use virtual text (at some point in the future) to display arg lists etc of the form you're hovering over, but I'm not fully sold into that just yet. (just like how cider/emacs display it in the bottom of the buffer, the minibuffer?)
Maybe this could be configurable / pluggable, I'll have a think about the UX of this. Inline results could be nice although that + HUD would be noisy, so you'd want one or the other, not both.
And then there's the question of: Could I display the arg lists as well as some results from evals if you opt into it. I'll give it some hammock time :smile:
I suppose in the spirit of "add a feature that benefits all clients", opting into inline results could work for every client out of the box.

Yeah, for sure. Take all the time you need. Honestly, I don't have a picture as big like you of how can this be. Thanks!

Hay, just switched to conjure. Wow it just gets out of your way and does exactly what you want. Thank you @Olical. Although I miss virtual text so much, no that I don't like the popup, but I'm used to having result apper on the buffer as I'm editing. conjure buffer I believe would work for me as repl.
Here's some implementations I liked: codi has an amazing expereience live eval lines or blocks as you type (far cry from what we actually want) but it has a great syntax highlighting for the results preview, acid.nvim 'virtual text implementation in lua' and iced-vim 'vimL'

Thanks for the support! And references! Virtual text is definitely on my todo list, I'll bear these ideas in mind when I get around to it. I'll share my plan for how it'll work here before it's implemented as a sort of RFC too :smile: hopefully it won't be too far away, I just wish I had more time!

Looking forward, Thank you. PS: getting my way around aniseed and configuring my nvim with it, hopefully soon I can contribute.

It's been a long time coming but I'm working on conjure.inline which is all about virtual text hints for things like documentation and eval results :eyes:

Initial pass working on the develop branch! No configuration or docs yet, so all evals end up in virtual text as well as the HUD. Need to work out something clever so you can have the following cases:

Only virtual, no HUD ever (unless requesting docs etc)
Only HUD, no virtual ever.
Hybrid, virtual for small eval results, HUD for multi line and docs / view source.

Requires time and experimentation!
The really neat thing is that this one implementation works for every Conjure client, so Racket, Fennel, Janet and Clojure all get the same feature :tada:

Oh thats really nice

Going to mark this as done and release it early even though I have more to do. I'll create a separate issue for the smart behaviour of when to use inline and when to use the HUD, for now both will appear on every eval result. I just want to get it released and in the user's hands so I can get feedback and confirmation that everything is working okay :blush:


Adds new events class into frontend toolkit.

This class allows users to easily set google event tracking on buttons and links across the Digital marketplace.
This has been copied over from the buyer frontend with an additional new feature to track a specific input or selection value.
Ticket: https://trello.com/c/FMuTKjo4/114-analytics-for-new-award-flow

We have some unit tests for trackEvent in the Buyer FE - is it possible to transfer these over as well?

hey @katstevens - doesn't seem any tests exist for the analytics stuff in the toolkit. I think it would be better to do that as a separate ticket as this is blocking some features going live for our mission. What do you think?

@DilwoarH That was my point 😸 There aren't any in the toolkit yet but we could potentially port over the ones from the buyer FE along with the code we've lifted from there. The tech debt ticket (https://trello.com/c/vgVAIG4n/233-shared-analytics-js-in-frontend-toolkit) has been lingering for ages because I wasn't sure how to get those tests working in the toolkit - I was hoping you'd have some ideas! If it's not straightforward then it's fine to leave it out of this PR.


Update docuemntation of =<

See #34

Coverage increased (+9.9%) to 43.773% when pulling e4c11669bf9d424881c93fac6fb32c5a39e54725 on doc-cats-transformers into 77000c5ce022ba8b99f3afbbf1aaae6a77784bf8 on master.

Coverage increased (+9.9%) to 43.773% when pulling e4c11669bf9d424881c93fac6fb32c5a39e54725 on doc-cats-transformers into 77000c5ce022ba8b99f3afbbf1aaae6a77784bf8 on master.

Coverage increased (+9.9%) to 43.773% when pulling e4c11669bf9d424881c93fac6fb32c5a39e54725 on doc-cats-transformers into 77000c5ce022ba8b99f3afbbf1aaae6a77784bf8 on master.


Minimality after MIMO Transfer and State Multiplication is still missing.

For the transfer multiplications, tests are performed and thanks to lcm and gcd stuff the order does not grow as drastically as is for matlab.
For State there is no minimality guarantee at the outset. Hence a minimalization step is necessary.
Maybe an option to automate that as a key as in G = F**H that results in simplify=True ? Needs a proper infix operator for this ** is just a placeholder.

The MIMO multiplication is done. However the infix operator issue is still open.

minimal_realization() works pretty stable for Transfers and States now. So this can be added after the multiplication.


Setup Discourse instance for platform discussion board

Probably easiest to have an empty set or unit instead within the system


Output Macro Expansions

I was thinking about how to make a nice auto completion thingy for Dale. And figured that it should be pretty easy to add a side module to the compilation and use the compile-time introspection to probe for a bunch of information like what types there are and what fields they have and what functions exist, where they are (so you can jump to definition).
One thing that probably seems impossible to do from the viewpoint of the program being compiled, is to find all the places where there are macro invocations, and get a text representation of what it was expanded to. This would be really useful information to have in your editor. Just select the code you have written and ask for what it looks like after expansion.
So either it should be possible to figure this out with macro expansion some how. Or a flag can be added to the compiler which spits out a file  that contains all the macro expansions in text for all the locations in the program. Which seems to me like the best option for that. But then it almost starts feeling like you may as well implement all these "IDE" tools in the compiler itself if you're already gonna do this. So not sure about the introspection approach (though it is cooler and nice to write it in Dale itself).

Seems like #110 is related to this issue. If that suggested way of implementing it I'm not sure how it would aid with this issue. Hmm. But either way, solving either issue should make it possible to share a lot of code for implementing the functionality wanted in these issues.

Or a flag can be added to the compiler which spits out a file that contains all the macro expansions in text for all the locations in the program. Which seems to me like the best option for that.

There is the --print-expansions compilation flag, which will cause all macro expansion details to be printed to standard output.  Does that look like it will do the job?

Hmm maybe! Would have to give it a run to see what it prints..

Gave it a test run with the macro example. I think this could do what I had in mind. I might do some experimentation with it when I have time!


Unify keyboard accelerators

With this update the keyboard accelerators are unified within BabyGruPreferences and most of them are handled within BabyGruKeyboardAccelerators. However:

The main bodies of some functions (move up/down left/right and take screenshot) are still located within mgWebGL. At the moment, this works by just calling these functions in BabyGruKeyboardAccelerators. I can refactor the whole functions into BabyGruKeyboardAccelerators but I thought it would be tidier to keep all the vec3 and quat4 stuff within mgWebGL. Let me know if you think otherwise. In contrast, I did refactor all of the "go to blob" and "clear labels" code into BabyGruKeyboardAccelerators.
Holding "m" and clicking on a residue is being taken care of within mgWebGL. To do this the attribute self.keysDown has been modified to contain information of which action can be performed based on the key mappings in BabyGruPreferences, which is now passed to mgWebGL via props.

@stuartjamesmcnicholas & @martinemnoble1 please let me know if I forgot to include another shortcut from mgWebGL or if you think there's a better way of doing this.

Now all the code has been moved into BabyGruKeyboardAccelerators. Let me know if you think this makes the code more messy.


Backport 14733 14024

part of #3416, but the rest has some test failures I have to work through

unable to replicate these test failures locally (my travis also didn't get these failures) so force pushed to retrigger CI

Alright... I have no idea why CI is mad

Resolved


Add support to move transfer funds related queries to GraphQL V2

Related to https://github.com/opencollective/opencollective/issues/4487
Front End PR: https://github.com/opencollective/opencollective-frontend/pull/6675

This is no longer needed as per https://github.com/opencollective/opencollective-frontend/pull/6675#discussion_r676039116. We can filter by type instead, so no need to add the service.

This is no longer needed as per opencollective/opencollective-frontend#6675 (comment). We can filter by type instead, so no need to add the service.

Actually we need this as we are using both filters, type and service as per, https://github.com/opencollective/opencollective-frontend/pull/6675#discussion_r676112597 😉


SPI Slave buffer write procedure inquiry

How does SPI slave handle the following:
User program 100 entry to SPI slave write buffer. Then he may realize one entry is wrong and need to correct it. (say entry 3).  What is the right procedure to do that? Can DV show us how to do that? With the current implementation, this can be a daunting process.

The best solution is not to write bad data into the FIFO. But if this does occur, the contents of the Write Buffer can be erased by issuing a soft reset. This should be as simple as writing a 0x3 to 0x44 which should be the write buffer control and the writing 0x0 to the same register.

Julie please verify and close this issue

Feature for future consideration.


chore(master): release 0.5.0

:robot: I have created a release beep boop
0.5.0 (2024-08-03)
Features

updated readme. (41cc623)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/tomshaw/electricgrid/releases/tag/v0.5.0 :sunflower:


Reset parameters button broken.

Reset parameters button is broken. When I clicked it the page transforms from

To

Without resetting the parameter values, only the formatting.
I'm on Chrome.

@MattHJensen are you working off of the test app? Or the main app? I'm having trouble re-creating this. Can you show what steps you took?

@brittainhard, I must have been on an edit parameters page. Now that I think about this more, the behavior was unexpected, but probably correct.
I'm going to move this issue out of 2.5 and think about if there's any way we can make this a little smoother for the users.

@brittainhard, I just had another user report this same issue, so I'm moving it back to 2.5.
Since the other user had the same reaction I did, I think the right behavior is:
if the user is on an edit parameter page, the reset parameters button will take them to a blank ospc.org/taxbrain page.

Fixed in #533


Inconsistent number of panes being created

Environment
Windows build number: Microsoft Windows [Version 10.0.18363.720]
Windows Terminal version (if applicable): 0.10.761.0

Steps to reproduce
When executing the following batch file multiple times an inconsistent number of panes is being created.
wt -p cmd ^
	; split-pane -H -p cmd  ^
 	; new-tab -p cmd ^
 	; split-pane -p cmd -H ^
 	; split-pane -p cmd -H ^
 	; split-pane -p cmd -H

Expected behavior
Two horizontal panes in tab one, four horizontal panes in tab two.
Actual behavior
2, 3 or 4 panes in tab two (inconsistently).

Thanks for the report! This is actually already being tracked by another issue on our repo - please refer to #4618 for more discussion.
/dup #4618


error

/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/actions/InteractionCreate.js:50
if (channel && !channel.isTextBased()) return;
^
TypeError: channel.isTextBased is not a function
at InteractionCreateAction.handle (/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/actions/InteractionCreate.js:50:33)
at Object.module.exports [as INTERACTION_CREATE] (/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/websocket/handlers/INTERACTION_CREATE.js:4:36)
at WebSocketManager.handlePacket (/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/websocket/WebSocketManager.js:352:31)
at WebSocketShard.onPacket (/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/websocket/WebSocketShard.js:489:22)
at WebSocketShard.onMessage (/home/runner/discord-js-bot-2/node_modules/discord.js/src/client/websocket/WebSocketShard.js:328:10)
at callListener (/home/runner/discord-js-bot-2/node_modules/ws/lib/event-target.js:290:14)
at WebSocket.onMessage (/home/runner/discord-js-bot-2/node_modules/ws/lib/event-target.js:209:9)
at WebSocket.emit (node:events:390:28)
at Receiver.receiverOnMessage (/home/runner/discord-js-bot-2/node_modules/ws/lib/websocket.js:1178:20)

I got this error too, I fixed it by updating discord.js to version 14.8.0 in package.json and npm i

WOAH ITS A BIG BUG TO REPORT

I got this error too, I fixed it by updating discord.js to version 14.8.0 in package.json and npm i

that should help

I got this error too, I fixed it by updating discord.js to version 14.8.0 in package.json and npm i

Thx It's work

Fixed as part of 5.3.0 release


Please update documentation as per current release of .NET Core and Identity server

Hello Team ,
thanks a lot for creating such a great tool/library for us and helping us implementing security for .net core application in micro-services environment.
This is to request you guys to please update the document as following it does not allow me to creating a working demo app as command and options mentioned here are not working as of now.
this makes learner loose confidence if he/she is going on with correct path or not.
thank you
Dheeraj Kumar

This is to request you guys to please update the document as following it does not allow me to creating a working demo app as command and options mentioned here are not working as of now.

Can you be more specific?

for example in overview of under Preparation you have command
dotnet new -i IdentityServer4.Templates
this does not run

I just ran the following five minutes ago it ran fine.

dotnet new -i IdentityServer4.Templates

What error did you get exactly?

Any update?

Works for me :)

Closing. If you still have issues, feel free to reopen.


Windows 10 (python example)

Some wrong with CDLL loading tdjson.dll
This is default python example, I just copy tdjson.dll and other files from build/debug folder to examples/python, and got this error (WinError 182)

cli.exe works fine
Builded with VS 2019. Python 3.7 x64

Probably, there is something wrong with the dependencies or with the built library. Could you try to use instructions from https://tdlib.github.io/td/build.html?language=Python to build 64-bit tdjson.dll from scratch. If this doesn't help then you can try to find the reason of the issue as suggested in https://stackoverflow.com/questions/3497301/what-exactly-means-error-invalid-ordinal.

@rejjin I hope that you have managed to successfully build TDLib using the build instructions generator.


Pool does not work with Drop types

When creating a Pool<T> where T: Drop, an attempt is made to drop zeroed memory, which may crash the program.
Running this example
extern crate pool;
fn main() {
    let _ = pool::Pool::with_capacity(1, 0, || std::thread::spawn(|| ()));
}

raises SIGTRAP on Linux. Backtrace:
Program received signal SIGTRAP, Trace/breakpoint trap.
0x000055555555c852 in std..thread..JoinHandle::drop.1453::h03e0f556ed6892ac ()
(gdb) bt
#0  0x000055555555c852 in std..thread..JoinHandle::drop.1453::h03e0f556ed6892ac ()
#1  0x000055555555e9be in pool..Entry$LT$std..thread..JoinHandle$GT$::drop.1809::h6a6f3ef0db7071dc ()
#2  0x000055555555b706 in jshs_test::Pool<T>::with_capacity<closure> (count=1, extra=0, init={struct JoinHandle (())} 0x7fffffffe600) at /home/josh/.cargo/registry/src/github.com-1ecc6299db9ec823/pool-0.1.1/src/lib.rs:76
#3  0x000055555555b31d in jshs_test::main () at src/main.rs:4
#4  0x0000555555573169 in rust_try_inner ()
#5  0x0000555555573156 in rust_try ()
#6  0x000055555557100f in rt::lang_start::h721c56921a1abf3du8I ()
#7  0x000055555555b485 in main ()

+1

Woops :smile: working on a fix.

Should be fixed on master: bdd7bd215d340ec85db5be1676489daabe789ba4.
If you can confirm that it is fixed for you, I can push a new version out.

It is still possible to cause a trap: If the initializer function panics at some point, not all entries will be initialized, but the Drop impl for PoolInner<T> tries to destroy all of them anyway.
Example, tested with c929ed480632f0ecdd88d9aa459b21487b2382fc:
extern crate pool;

#[derive(Clone, Default)]
struct Test;

impl Drop for Test {
    fn drop(&mut self) { }
}

fn main() {
    let _ = pool::Pool::<Test>::with_capacity(1, 0, || panic!("oops"));
}

Good call. Hopefully it's good now!

:+1: Looks fine, thanks!

I pushed a new crate w/ the fixes: v0.1.2


Very slow to update nodes when starting up Tokens

Describe the bug
A clear and concise description of what the bug is.
To Reproduce

Start up Figma tokens
Click the unchecked Foundation and Web sub folder
The plugin starts fetching and updating nodes, and it takes a few minutes to complete
I'm on a M2 Pro Max and on a fiber network so performance on this side should not be the issue.
Also my colleagues pulling from same files do not have this issue, although they're not working in this same file, which might be getting to big or complex?

Expected behavior
being Instantly able to use tokens plugin without the issue
Screenshots or Screencasts

Figma file (optional)
Cant share this file, sorry, let me know if there is any info i can share to help
JSON (optional)
If possible, attach the JSON that leads to this bug.

So in your case, you:

Start up the plugin
Activate a few themes
Notice slowness

Correct?
In that case, I'd recommend to change the Apply to settings in the bottom right to Apply to selection. I assume you are working in a huge document, and have this set to Apply to document (or Page and are working in a big page).
In your screenshot the plugin needs to iterate over 19k nodes, which will take a while and is expected.
As long as you do not activate / deactivate sets you are able to use the plugin instantly, but any "check" of a set / folder causes the plugin to update the selection / page / document (whatever you chose).
Closing this as I think this solves your problem, but feel free to reopen if you think this is a mistake.


execute-free apk installation

Ability to run apk on non-Linux platforms, without chroot/proot, without qemu, without root, without installed utilities.
This is pretty hefty - it might be better broken down into multiple PRs (or at least commits), I am open to suggestions - so I will try and break it down:

apk implementation: all in pkg/apk/impl/. This intentionally is built as something that we could remove, either upstream to alpine-go or to a separate library of our own. It is consumed by pkg/apk/. This replaces all apk exec calls.
rwfs: an interface defined an extension of fs.FS that supports reading, writing, mknod, chmod, chown
rwosfs: an implementation of rwfs that uses an underlying filesystem dir for implementation; similar to os.DirFS for fs.FS.
an update to the goheader header.tmpl to support any whitespace before the apache URL; some editors change the format and it fails on tab vs space vs 2 space vs space+tab vs I have no idea what. This makes it much more tolerant.
updates to pkg/vfs/ so it will work on non-Linux platforms, or at least compile.

There is some extensive documentation in docs/apk.md.
Let the comments begin. 😄
Something important: I modified the original version of the initdb part based on the work in #412. Thanks to @puerco's PR for some good ideas there.

I already pulled the header template changes into their own PR #427

There are some arbitrary or hard-coded things that could use some flexibility - like the base OS version, or alpine-keys version. I would be happy to get some good ideas on the best approach for those.

Ah, nice. go tests and lint are clean.

This looks good as a first cut, but one thing I noticed is missing is an implementation of the solver algorithm.  Am I just missing it, or is this implementation unable to solve the full DAG given an /etc/apk/world file still?

@Ikke great to meet you. If you have a moment, can you look at what is in here, specifically under pkg/apk/impl? All of that (incomplete but mostly functional) implementation of local apk commands. It is built to be extracted from this project into its own, whether alpine-go or a separate library? See what is appropriate for being part of official alpine go libs?

I think a lot of this can make its way into alpine-go (if desired), but I think it can treated as a separate task after this is merged.

I see one lint and one lint/codeql errors, not sure what to do with them.

lint is complaining about the copyrights; I fixed them on every file we changed, and running golangci-lint run locally shows no copyright errors
both lint and codeql are complaining about the symlink arbitrary file write, but it already was evaluated

Actually, it is odder than that. EvalSymlinks will not help, as it validates them against the local filesystem, but we are using them internally in an fs.FS. So it doesn't do any good, and actually fails.
I think we may need to bypass it, unless someone has a better idea?

Bypassing it is probably fine for now.

On an M1 mac, and ran the following:
mkdir tmp
cd tmp
git clone https://github.com/deitch/apko.git -b apk-cli-free .
make apko
curl -sLO https://raw.githubusercontent.com/chainguard-images/images/main/images/wolfi-base/configs/latest.apko.yaml
./apko build latest.apko.yaml wolfi-base output.tar

and get an error related to symlinks:
Jan  3 17:17:49.041 [INFO] [arch:aarch64] synchronizing with desired apk world
Error: failed to build layer image: initializing apk: failed to fixate apk world: unable to install files for pkg wolfi-baselayout: invalid symlink /proc/mounts: lstat /proc: no such file or directory
2023/01/03 17:17:49 error during command execution: failed to build layer image: initializing apk: failed to fixate apk world: unable to install files for pkg wolfi-baselayout: invalid symlink /proc/mounts: lstat /proc: no such file or directory

@jdolitsky yeah, it was that EvalSymlinks() that caused the issue. It evaluates in the current filesystem, which is not what we want. I removed it and pushed out, so pull the branch and try again please?

@kaniini do you recall the "ignore this syntax" offhand?

@deitch thanks. after re-doing above commands I get passed the symlink error but now have:
Jan  4 10:13:38.279 [INFO] [arch:aarch64] synchronizing with desired apk world
Error: failed to build layer image: initializing apk: failed to fixate apk world: unable to update scripts.tar for pkg glibc: unable to write scripts header for glibc-2.36-r3.Q1/ido5cSLfh1lALe6GCoMfKXSf6Q=.trigger: archive/tar: cannot encode header: Format specifies USTAR; and USTAR cannot encode ChangeTime=1969-12-31 18:00:00 -0600 CST
2023/01/04 10:13:39 error during command execution: failed to build layer image: initializing apk: failed to fixate apk world: unable to update scripts.tar for pkg glibc: unable to write scripts header for glibc-2.36-r3.Q1/ido5cSLfh1lALe6GCoMfKXSf6Q=.trigger: archive/tar: cannot encode header: Format specifies USTAR; and USTAR cannot encode ChangeTime=1969-12-31 18:00:00 -0600 CST

however works fine with alpine-base !!

using https://raw.githubusercontent.com/chainguard-images/images/main/images/alpine-base/configs/latest.apko.yaml
and doing docker load we have a different issue:
$ docker -D load < output.tar
492d1793c47d: Loading layer [==================================================>]  3.249MB/3.249MB
Error processing tar file(duplicates of file paths not supported):

@jdolitsky take another look w/r/t the USTar issue. I will look at the docker load next. Docker really should be happy to accept sequential tar files that overwrite, but we can fix that if it does not.

The mink failure is because of riscv64, which has to do with the different key verification algorithm. Hopefully we will know what that is soon enough.

The mink failure is because of riscv64, which has to do with the different key verification algorithm. Hopefully we will know what that is soon enough.

Is it the size that is the issue (2048-bits instead of 4096)? Because if so, there are still packages on edge that are signed with the 2048-bits keys.

@Ikke here you go, this gist. I must be missing something

update - wolfi-base appears to build, but the docker load still failing with the same

update - wolfi-base appears to build, but the docker load still failing with the same

I had no problem with the load, I
$ curl -sL https://raw.githubusercontent.com/chainguard-images/images/main/images/alpine-base/configs/latest.apko.yaml -o /tmp/alpine-latest.apko.yaml
$ go run . build /tmp/alpine-latest.apko.yaml alpine-base /tmp/alpine-base.tar
$ cat /tmp/alpine-base.tar | docker -D load
$

Same if I build with https://raw.githubusercontent.com/chainguard-images/images/main/images/wolfi-base/configs/latest.apko.yaml

Had to rebase with a lot of things, but seems to be back in the right place.
As far as I can tell, we have two outstanding issues:

The signature verification issue with riscv64; hopefully with the gist, @Ikke will be able to shed some light on it.
The docker load issue; hopefully @jdolitsky will be able to replicate it.

@Ikke here you go, this gist. I must be missing something

Not anything concrete yet, but if I add ppc64le (which happens to use 2048-bits keys atm), it fails as well, so it's something to do with 2048-bits keys, but not sure why that would be failing yet.

Yeah, strange, is it not? Probably something tiny that we are just missing.

OK, I think I have something.
I changed the calculation of what to use for the hash (and then compared to the signature) to this:
        allBytes := len(b)
        unreadBytes := buf.Len()
        readBytes := allBytes - unreadBytes
        indexData := b[readBytes:]

Same functionality, but it gives me the ability to inspect the byte count. I got the following:

aarch64 (works): readBytes = 666
riscv64 (fails): readBytes = 391

I think tried chopping off the first 666 (aarch64) or 391 (riscv64) bytes from each file and seeing if it still is valid tgz file, which it should be. First aarch64, which works:
$ dd if=APKINDEX-aarch64.tar.gz of=APKINDEX-aarch64-part.tar.gz ibs=1 skip=666
656533+0 records in
1282+1 records out
656533 bytes transferred in 0.358321 secs (1832248 bytes/sec)
$ ls -la *aarch*
-rw-r--r--  1 adeitcher  wheel  656533 Jan  8 17:54 APKINDEX-aarch64-part.tar.gz
-rw-r--r--  1 adeitcher  wheel  657199 Jan  8 17:26 APKINDEX-aarch64.tar.gz
$ tar tvf APKINDEX-aarch64-part.tar.gz
-rw-r--r--  0 root   root       24 Jan  8 14:28 DESCRIPTION
-rw-r--r--  0 root   root  1951912 Jan  8 14:28 APKINDEX

That looks good.
Next riscv64 (which fails):
$ dd if=APKINDEX-riscv64.tar.gz of=APKINDEX-riscv64-part.tar.gz ibs=1 skip=391
643376+0 records in
1256+1 records out
643376 bytes transferred in 0.359835 secs (1787975 bytes/sec)
$ ls -l *riscv*
-rw-r--r--  1 adeitcher  wheel  643376 Jan  8 17:53 APKINDEX-riscv64-part.tar.gz
-rw-r--r--  1 adeitcher  wheel  643767 Jan  8 17:26 APKINDEX-riscv64.tar.gz
$ tar tvf APKINDEX-riscv64-part.tar.gz
tar: Error opening archive: Unrecognized archive format

Oops. Something about either the calculation, or how we are using go to read the unread bytes, is failing.
I am pretty sure the logic is correct. Treat it as two concatenated gzip streams, read the first as the signature, lop it off, hash the rest, verify the signature. It is the "calculation of lop it off" that is wrong.
I then used xxd to look at the file in that area for the gzip stream typical header magic number 0x1f8b. I found it at 399 instead of 391. So I tried again:
$ dd if=APKINDEX-riscv64.tar.gz of=APKINDEX-riscv64-part.tar.gz ibs=1 skip=399
643368+0 records in
1256+1 records out
643368 bytes transferred in 0.364523 secs (1764959 bytes/sec)
$ tar tvf APKINDEX-riscv64-part.tar.gz
-rw-r--r--  0 root   root       24 Jan  8 14:28 DESCRIPTION
-rw-r--r--  0 root   root  1920522 Jan  8 14:28 APKINDEX

That worked! OK, so the calculation is off by 8 bytes. But why and how?

Yup. And if I force it with:
        if readBytes == 391 {
                readBytes = 399
        }

then it all works. So this is a gzip stream reading error. Figure that out, and it works. Still no idea why, though.

More. I stepped through the code, checking the buf.off which is the offset, essentially the amount read. I get the following.
aarch64:

after signatureFile, err := tarReader.Next(): 658
after signature, err := io.ReadAll(tarReader): 666

riscv64:

after signatureFile, err := tarReader.Next(): 391
after signature, err := io.ReadAll(tarReader): 391

Some sort of caching it is doing? 🤷♂

Aha! Got it! Successful day! Unless you call Next() and check for an EOF (with multistream), it will not complete the reading. We simply got lucky with the longer ones. Try the gist now.

Phew. riscv64 works now.
@jdolitsky , tell me how to replicate the docker load issue - exactly what build it was - and I will figure that out.

Aha! Got it! Successful day! Unless you call Next() and check for an EOF (with multistream), it will not complete the reading. We simply got lucky with the longer ones. Try the gist now.

Ah yes, nice find!

mink test is failing. I cannot quite figure out what build command it is executing, as it does not seem to list anywhere, "this is the command I am going to execute".
I think it is apko publish something, but no really sure. Can someone describe what it is?

regarding the docker load issue, here's how I reproduce (similar to comment here)
mkdir tmp
cd tmp
git clone https://github.com/deitch/apko.git -b apk-cli-free .
make apko
curl -sLO https://raw.githubusercontent.com/chainguard-images/images/main/images/wolfi-base/configs/latest.apko.yaml
./apko build latest.apko.yaml wolfi-base output.tar
docker -D load < output.tar

Error:
a53226dbe638: Loading layer [==================================================>]  4.677MB/4.677MB
Error processing tar file(duplicates of file paths not supported):

In fact, even if I follow your instructions here I get the same error.. maybe we can sit down this week to look at it if needed. I'm using Docker Desktop on M1 mac

mink error seems to be part of the apko build:
  Error: failed to build layer image for "arm/v7": failed to generate tarball for image: writing TAR archive failed: mknod not supported by this fs: path (dev/console)
  2023/01/08 17:56:43 error during command execution: failed to build layer image for "arm/v7": failed to generate tarball for image: writing TAR archive failed: mknod not supported by this fs: path (dev/console)

unclear to me right now what that means.. but perhaps indicates a difference in how tar is built

In fact, even if I follow your instructions https://github.com/chainguard-dev/apko/pull/426#issuecomment-1374563269 I get the same error.. maybe we can sit down this week to look at it if needed. I'm using Docker Desktop on M1 Mac

My Mac has docker with experimental containerd backend support, so it is possible mine doesn't have an issue with duplicate paths. But I managed to reproduce it on a different installation.

mink error seems to be part of the apko build
unclear to me right now what that means.. but perhaps indicates a difference in how tar is built

I think it calls apko publish, and thus does a slightly different code path. I am unsure, maybe @kaniini knows? If I could get the proper invocation, I can track it down.

My guess is the mink task definition is here: https://github.com/chainguard-dev/apko/blob/main/config/task.yaml#L45

mknod needs to be supported (via faking) for all FSes.  we have to support anything that can be in a tar stream.

@jdolitsky can you try the docker load again with the latest commits here? I reworked it.

@deitch the load works fine with wolfi-base!

Was trying to build other images to see if the image can run, and ran into another issue at least for the following 2 configs:

https://raw.githubusercontent.com/chainguard-images/images/main/images/python/configs/latest-dev.apko.yaml
https://raw.githubusercontent.com/chainguard-images/images/main/images/ruby/configs/latest-3.2.apko.yaml

Error: failed to build layer image: initializing apk: failed to fixate apk world: unable to install files for pkg ncurses-terminfo-base: unsupported file type 49
2023/01/09 17:06:54 error during command execution: failed to build layer image: initializing apk: failed to fixate apk world: unable to install files for pkg ncurses-terminfo-base: unsupported file type 49

The melange source for that wolfi package is here: https://github.com/wolfi-dev/os/blob/main/ncurses.yaml
and .apk can be found here: https://packages.wolfi.dev/os/aarch64/ncurses-terminfo-base-6.3-r2.apk

With #458 now in, I rebased it so it can use those checks, but removed the tests that build the examples in docker; after all, the whole purpose of this PR is to run it all natively without calling out.

This almost is good. One build image is failing, ./examples/wolfi-base.yaml. The reason is as follows. The order of packages to be installed is:

wolfi-keys
glibc
busybox
lib
libcrypto3
libssl3
wolfi-baselayout

The problem is that the second one, glibc, installs /lib64 as a directory and puts some things in there, while wolfi-baselayout tries to create a symlink /lib64 -> /lib. That fails because /lib64 already exists as a directory.
I am not sure how this worked in the past, should it not have failed? Does it ignore the link? Or did the ordering here get messed up?
FWIW, I ran cgr.dev/chainguard/wolfi-base and it has /lib64 -> /lib.

While looking at it, in the actual image now:
# cat /etc/apk/world
ca-certificates-bundle
wolfi-base
wolfi-baselayout

while the example has:
contents:
  keyring:
    - https://packages.wolfi.dev/os/wolfi-signing.rsa.pub
  repositories:
    - https://packages.wolfi.dev/os
  packages:
    - ca-certificates-bundle
    - wolfi-base

The dependency tree looks as follows (I walked it through manually based on https://packages.wolfi.dev/os/x86_64/APKINDEX.tar.gz)

ca-certificates-bundle :
wolfi-base

apk-tools

ca-certificates-bundle
glibc

wolfi-baselayout

libcrypto3
libssl3
zlib

busybox
wolfi-keys

So wolfi-base -> glibc -> wolfi-baselayout.
That means that it should have executed wolfi-baselayout first, which would make the lib64 symlink exist. Ordering clearly is a bit off, but we will get it.

I've gone ahead and marked the CodeQL alerts as "false positive"

I've gone ahead and marked the CodeQL alerts as "false positive"

Thanks Josh. I would mark it inline in the code, but apparently codeql does not support it 🤷♂

Going ahead and merging this, and lets see what breaks.