Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The log cannot show the real IP of the client #236

Closed
ac169 opened this issue Jun 26, 2023 · 14 comments · Fixed by #245
Closed

The log cannot show the real IP of the client #236

ac169 opened this issue Jun 26, 2023 · 14 comments · Fixed by #245
Assignees
Labels
bug Something isn't working

Comments

@ac169
Copy link

ac169 commented Jun 26, 2023

Jun 26 03:31:02 co webp-server[553824]: 03:31:01 | 200 | 1ms | 127.0.0.1 | GET | /image/cache/P-DJI-T30-010-560x560.jpg
Jun 26 03:31:03 co webp-server[553824]: 03:31:03 | 200 | 2ms | 127.0.0.1 | GET | /image/catalog/P-DJI-T30-010.jpg
Jun 26 03:32:30 co webp-server[553824]: 03:32:29 | 200 | 0s | 127.0.0.1 | GET | /image/cache/catalog/P-DJI-T30-011-100x100.jpg

When the webp_server_go is used as the Nginx back end, even if the proxy_set_header X-Real-IP $ remote_addr, etc. are added to the nginx configuration file, there is no real IP in the log in the log. Not support Is there a problem with the real IP of the client? If the webp_server_go log itself does not support the significance of displaying the real IP of the client, what is the significance of the log?

@BennyThink BennyThink self-assigned this Jun 26, 2023
@BennyThink BennyThink added the bug Something isn't working label Jun 26, 2023
@BennyThink
Copy link
Member

var app = fiber.New(fiber.Config{
	ServerHeader: "WebPPT",
	ProxyHeader:  "X-Real-IP", 
	AppName:      "WebPPT",
})

I think we should set that ProxyHeader.

@ac169
Copy link
Author

ac169 commented Jun 26, 2023

var app = fiber.New(fiber.Config{
	ServerHeader: "WebPPT",
	ProxyHeader:  "X-Real-IP", 
	AppName:      "WebPPT",
})

I think we should set that ProxyHeader.

I haven't learned and used Golang, so I can't understand the code. Nginx configuration is as follows:


 ......

 location ~* (\.(jpg|jpeg|bmp|png))$ {
        proxy_pass http://127.0.0.1:6666;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_hide_header X-Powered-By;
        proxy_set_header HOST $http_host;
        expires max;
        access_log off;
    }

 ......

Excuse me, is it caused by the error of the true IP of the client?

@BennyThink
Copy link
Member

Your configuration is correct, nginx will set header X-Real-IP with $remote_addr. We'll add this header soon.

BennyThink added a commit that referenced this issue Jun 26, 2023
@BennyThink
Copy link
Member

this issue will be resolved by merging this PR #220

@ac169
Copy link
Author

ac169 commented Jun 27, 2023

/lib64/libc.so.6: version `GLIBC_2.34' not found

In addition, I suggest that the Release version of the Release version of the Release version is compatible with the older system and GLIBC version. Of course, users can also compile them by themselves, but after all, the high version can be compiled by the low version GLIBC, and the low version GLIBC cannot run the high version of GLIBC Application of compilation. So I always feel less perfect

@BennyThink
Copy link
Member

BennyThink commented Jun 27, 2023

Yes you're right. We're planning to change the release_binary workflow to a lower version of distros.

Current considerations:

  • centos 7: release on 2014, old enough, but we observed panic issue when using libvips, also it's not officially in golang's docker image ❓ TO BE CONFIRMED
  • debian stretch: released on 2017, quiet old, but we're unable to install libvips even we switch to debian archive image
  • debian buster: released on 2019, official golang image support, can install libvips-dev
  • ubuntu 18.04: also old enough, can install libvips, but official apt only supports golang 1.10 which is incompatible with go module, we require 1.11 and above. Of course we can install golang from go.dev

Personally I would prefer to use debian buster. Of course we would be appreciated if you can help us to resolve dependencies issues for stretch.

@n0vad3v what's your idea on this?

@BennyThink
Copy link
Member

Update:

  • centos: it's too complicated to install libvips-devel on centos, so the choices are:
  • use ubuntu 20.04 as runner os to build binaries directly: could not determine kind of name for C.VIPS_INTERESTING_ALL, need to install from ppa
  • use any runner os, start a container in bullseye or 1804, build binaries inside container. TO BE TESTED

@ac169
Copy link
Author

ac169 commented Jun 27, 2023

The installation of libvips on the old version of the system is not complicated. You can refer to the connection https://tufora.com/tutorials/linux/general/install-vips-vips-tools-and-vips-devel-libvips-on-centos-7. In fact, this problem with libvips installation is secondary. The main problem is: if the Golang code is compiled, if the parameter does not specify the default, the GLIBC library that is dynamically referenced by the current compilation environment will be referenced and associated with its version, then an error will be reported when the actual run. Users can compile on their own platform to solve the GLIBC library version problem. This is only a suggestion for compatibility.

@n0vad3v
Copy link
Member

n0vad3v commented Jun 27, 2023

@ac169 Thanks for suggestion, indeed using an older version of OS for compile should make the binary work on more OSs.

If you have any ideas on how to archive this, free feel to help us by contributing to https://github.com/webp-sh/webp_server_go/blob/master/.github/workflows/release_binary.yaml .

Currently on our tests on CentOS 7 with the article you've suggested, we've meet the following errors:

--> Finished Dependency Resolution
Error: Package: vips-devel-8.14.2-1.el7.remi.x86_64 (remi-safe)
           Requires: pkgconfig(libwebpmux) >= 0.6
           Available: libwebp-devel-0.3.0-7.el7.i686 (base)
               pkgconfig(libwebpmux) = 0.3.0
           Available: libwebp-devel-0.3.0-10.el7_9.i686 (updates)
               pkgconfig(libwebpmux) = 0.3.0
           Available: libwebp-devel-0.3.0-11.el7.i686 (updates)
               pkgconfig(libwebpmux) = 0.3.0
Error: Package: vips-devel-8.14.2-1.el7.remi.x86_64 (remi-safe)
           Requires: pkgconfig(libwebpdemux) >= 0.6
Error: Package: vips-devel-8.14.2-1.el7.remi.x86_64 (remi-safe)
           Requires: pkgconfig(libwebp) >= 0.6
           Available: libwebp-devel-0.3.0-7.el7.i686 (base)
               pkgconfig(libwebp) = 0.3.0
           Available: libwebp-devel-0.3.0-10.el7_9.i686 (updates)
               pkgconfig(libwebp) = 0.3.0
           Available: libwebp-devel-0.3.0-11.el7.i686 (updates)
               pkgconfig(libwebp) = 0.3.0
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest

Since playing with different types of OS is a little bit time consuming, our current primary goal now might be fixing bugs and continue our refactor, as there are two bugs are waiting to be fixed and a feature created by @bugfest (#226) haven't been reviewed and discussed.

We'll mark this a low-priority but important task now, and will be take in to consideration after we've done things above, and currently it's suggested to use Docker for running if your system has an older GLIBC.

BennyThink added a commit that referenced this issue Jun 27, 2023
* runnable

* convert is working

* some refactoring

* update go.mod

* fix some TODOs

* add TODO

* update go mod

* rebase onto master

* fix #234

2: 5.9s - 7.6MB
4: 26s - 6.9MB

* fix malloc tests

* fix malloc tests

* remote TODO

* add X-Real-IP #236

* Better localRawImagePath

* remove some wrong comments

* Bump version to 0.9.0

---------

Co-authored-by: n0vad3v <n0vad3v@riseup.net>
@ac169
Copy link
Author

ac169 commented Jun 27, 2023

CentOS 7.9.2009
5.4.246-1.el7.elrepo.x86_64
GLIBC_2.17

The method introduced before is that I have verified it myself, and now the application is running on the old version system!

The problem should be on the libwebp library. You use the official libwebp-devel-0.3.0-7.el7 library. I automatically installed the libwebp7-devel-1.0.3-1.el7.remi.x86_64 library from the Remi source in the previous way.

The reason may be that the libwebp library has been installed before installing the vips-devel library

@BennyThink
Copy link
Member

BennyThink commented Jun 27, 2023

Key points:

  1. we need libvips 8.10+
  2. we need libwebp 0.6.0+, required by libvips
  3. we want to support both amd64 and arm64, but remi's repo only supports amd64

I create an experimental repo https://github.com/webp-sh/libvips . It runs on CentOS 7 with ancient version of glibc. However, even if you get the binary built by that system, you'll still need to install libvips on your own system, and its version should be above 8.10.

In conclusion:

  1. we will use CentOS 7 to build binary @n0vad3v , user still need to figure out how to install or build libvips.
  2. it's strongly recommended to run it in docker

@n0vad3v
Copy link
Member

n0vad3v commented Jul 2, 2023

We've written a blog post for this: libvips, CGO, and purego: How to Compile and Run Go Applications on different platforms.

@bugfest
Copy link
Contributor

bugfest commented Jul 2, 2023

Kudos for the blog post, it's a master piece! Have you considered providing srpm / spec files for these users so they can easily build these required packages?

@n0vad3v
Copy link
Member

n0vad3v commented Jul 2, 2023

Glad you like it @bugfest , I think currently we probably aren't going to provide srpm / spec files for them as we want to keep the release simple, so we're going to prioritize support for multi-platform binaries and docker images for now.

BennyThink added a commit that referenced this issue Jul 3, 2023
@BennyThink BennyThink assigned n0vad3v and unassigned BennyThink Jul 3, 2023
n0vad3v pushed a commit that referenced this issue Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants