Skip to content

Commit

Permalink
fix: incorrect request headers in proxy mode and deleted unparsable c…
Browse files Browse the repository at this point in the history 
…ached data (#719)

proxy时因为一个低级的拼写错误没有正确的携带请求头，导致代理模式时返回的数据不正确。但是现在用户发起的请求中的user-agent和x-forwarded等头部信息也没有正确的携带。虽然影响不大但还是想和跑批时更新的请求做一下区分。


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Enhanced error handling and logging for task execution, improving
traceability.
  
- **Improvements**
- Updated HTTP header access method for better alignment with context
structure.
- Clarified logic for manifest retrieval based on file type, ensuring
correct API usage.
- Streamlined cache handling and response generation logic in package
management.
- Improved method visibility and organization within the cache service
and controller.
- Simplified task creation logic and cache removal processes in the
controller.
- Updated expected outcomes for cache-related operations in the test
cases.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
@hezhengxu2018
hezhengxu2018 authored Dec 23, 2024
1 parent 638a3da commit 2780c53
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 183 deletions.
133 changes: 77 additions & 56 deletions app/core/service/ProxyCacheService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,28 @@ export class ProxyCacheService extends AbstractService {
}

async getPackageManifest(fullname: string, fileType: DIST_NAMES.FULL_MANIFESTS| DIST_NAMES.ABBREVIATED_MANIFESTS): Promise<AbbreviatedPackageManifestType|PackageManifestType> {
const isFullManifests = fileType === DIST_NAMES.FULL_MANIFESTS;
const cachedStoreKey = (await this.proxyCacheRepository.findProxyCache(fullname, fileType))?.filePath;
if (cachedStoreKey) {
const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey);
const nfsString = Buffer.from(nfsBytes!).toString();
const nfsPkgManifgest = JSON.parse(nfsString);
return nfsPkgManifgest;
try {
const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey);
if (!nfsBytes) throw new Error('not found proxy cache, try again later.');

const nfsBuffer = Buffer.from(nfsBytes);
const { shasum: etag } = await calculateIntegrity(nfsBytes);
await this.cacheService.savePackageEtagAndManifests(fullname, isFullManifests, etag, nfsBuffer);

const nfsString = nfsBuffer.toString();
const nfsPkgManifest = JSON.parse(nfsString);
return nfsPkgManifest as AbbreviatedPackageManifestType|PackageManifestType;
} catch (error) {
/* c8 ignore next 6 */
if (error.message.includes('not found proxy cache') || error.message.includes('Unexpected token : in JSON at')) {
await this.nfsAdapter.remove(cachedStoreKey);
await this.proxyCacheRepository.removeProxyCache(fullname, fileType);
}
throw error;
}
}

const manifest = await this.getRewrittenManifest<typeof fileType>(fullname, fileType);
Expand All @@ -88,9 +104,19 @@ export class ProxyCacheService extends AbstractService {
}
const cachedStoreKey = (await this.proxyCacheRepository.findProxyCache(fullname, fileType, version))?.filePath;
if (cachedStoreKey) {
const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey);
const nfsString = Buffer.from(nfsBytes!).toString();
return JSON.parse(nfsString) as PackageJSONType | AbbreviatedPackageJSONType;
try {
const nfsBytes = await this.nfsAdapter.getBytes(cachedStoreKey);
if (!nfsBytes) throw new Error('not found proxy cache, try again later.');
const nfsString = Buffer.from(nfsBytes!).toString();
return JSON.parse(nfsString) as PackageJSONType | AbbreviatedPackageJSONType;
} catch (error) {
/* c8 ignore next 6 */
if (error.message.includes('not found proxy cache') || error.message.includes('Unexpected token : in JSON at')) {
await this.nfsAdapter.remove(cachedStoreKey);
await this.proxyCacheRepository.removeProxyCache(fullname, fileType);
}
throw error;
}
}
const manifest = await this.getRewrittenManifest(fullname, fileType, versionOrTag);
this.backgroundTaskHelper.run(async () => {
Expand All @@ -109,6 +135,27 @@ export class ProxyCacheService extends AbstractService {
await this.proxyCacheRepository.removeProxyCache(fullname, fileType, version);
}

replaceTarballUrl<T extends DIST_NAMES>(manifest: GetSourceManifestAndCacheReturnType<T>, fileType: T) {
const { sourceRegistry, registry } = this.config.cnpmcore;
if (isPkgManifest(fileType)) {
// pkg manifest
const versionMap = (manifest as AbbreviatedPackageManifestType|PackageManifestType)?.versions;
for (const key in versionMap) {
const versionItem = versionMap[key];
if (versionItem?.dist?.tarball) {
versionItem.dist.tarball = versionItem.dist.tarball.replace(sourceRegistry, registry);
}
}
} else {
// pkg version manifest
const distItem = (manifest as AbbreviatedPackageJSONType | PackageJSONType).dist;
if (distItem?.tarball) {
distItem.tarball = distItem.tarball.replace(sourceRegistry, registry);
}
}
return manifest;
}

async createTask(targetName: string, options: UpdateProxyCacheTaskOptions): Promise<CreateUpdateProxyCacheTask> {
return await this.taskService.createTask(Task.createUpdateProxyCache(targetName, options), false) as CreateUpdateProxyCacheTask;
}
Expand Down Expand Up @@ -151,44 +198,37 @@ export class ProxyCacheService extends AbstractService {
await this.taskService.finishTask(task, TaskState.Success, logs.join('\n'));
}

async getRewrittenManifest<T extends DIST_NAMES>(fullname:string, fileType: T, versionOrTag?:string): Promise<GetSourceManifestAndCacheReturnType<T>> {
// only used by schedule task
private async getRewrittenManifest<T extends DIST_NAMES>(fullname:string, fileType: T, versionOrTag?:string): Promise<GetSourceManifestAndCacheReturnType<T>> {
let responseResult;
const USER_AGENT = 'npm_service.cnpmjs.org/cnpmcore';
switch (fileType) {
case DIST_NAMES.FULL_MANIFESTS:
responseResult = await this.getUpstreamFullManifests(fullname);
case DIST_NAMES.FULL_MANIFESTS: {
const url = `/${encodeURIComponent(fullname)}?t=${Date.now()}&cache=0`;
responseResult = await this.getProxyResponse({ url, headers: { accept: 'application/json', 'user-agent': USER_AGENT } }, { dataType: 'json' });
break;
case DIST_NAMES.ABBREVIATED_MANIFESTS:
responseResult = await this.getUpstreamAbbreviatedManifests(fullname);
}
case DIST_NAMES.ABBREVIATED_MANIFESTS: {
const url = `/${encodeURIComponent(fullname)}?t=${Date.now()}&cache=0`;
responseResult = await this.getProxyResponse({ url, headers: { accept: ABBREVIATED_META_TYPE, 'user-agent': USER_AGENT } }, { dataType: 'json' });
break;
case DIST_NAMES.MANIFEST:
responseResult = await this.getUpstreamPackageVersionManifest(fullname, versionOrTag!);
}
case DIST_NAMES.MANIFEST: {
const url = `/${encodeURIComponent(fullname)}/${encodeURIComponent(versionOrTag!)}`;
responseResult = await this.getProxyResponse({ url, headers: { accept: 'application/json', 'user-agent': USER_AGENT } }, { dataType: 'json' });
break;
case DIST_NAMES.ABBREVIATED:
responseResult = await this.getUpstreamAbbreviatedPackageVersionManifest(fullname, versionOrTag!);
}
case DIST_NAMES.ABBREVIATED: {
const url = `/${encodeURIComponent(fullname)}/${encodeURIComponent(versionOrTag!)}`;
responseResult = await this.getProxyResponse({ url, headers: { accept: ABBREVIATED_META_TYPE, 'user-agent': USER_AGENT } }, { dataType: 'json' });
break;
}
default:
break;
}

// replace tarball url
const manifest = responseResult.data;
const { sourceRegistry, registry } = this.config.cnpmcore;
if (isPkgManifest(fileType)) {
// pkg manifest
const versionMap = manifest.versions || {};
for (const key in versionMap) {
const versionItem = versionMap[key];
if (versionItem?.dist?.tarball) {
versionItem.dist.tarball = versionItem.dist.tarball.replace(sourceRegistry, registry);
}
}
} else {
// pkg version manifest
const distItem = manifest.dist || {};
if (distItem.tarball) {
distItem.tarball = distItem.tarball.replace(sourceRegistry, registry);
}
}
const manifest = this.replaceTarballUrl(responseResult.data, fileType);
return manifest;
}

Expand All @@ -204,7 +244,7 @@ export class ProxyCacheService extends AbstractService {
await this.nfsAdapter.uploadBytes(storeKey, nfsBytes);
}

private async getProxyResponse(ctx: Partial<EggContext>, options?: HttpClientRequestOptions): Promise<HttpClientResponse> {
async getProxyResponse(ctx: Partial<EggContext>, options?: HttpClientRequestOptions): Promise<HttpClientResponse> {
const registry = this.npmRegistry.registry;
const remoteAuthToken = await this.registryManagerService.getAuthTokenByRegistryHost(registry);
const authorization = this.npmRegistry.genAuthorizationHeader(remoteAuthToken);
Expand All @@ -221,8 +261,8 @@ export class ProxyCacheService extends AbstractService {
compressed: true,
...options,
headers: {
accept: ctx.header?.accept,
'user-agent': ctx.header?.['user-agent'],
accept: ctx.headers?.accept,
'user-agent': ctx.headers?.['user-agent'],
authorization,
'x-forwarded-for': ctx?.ip,
via: `1.1, ${this.config.cnpmcore.registry}`,
Expand All @@ -231,23 +271,4 @@ export class ProxyCacheService extends AbstractService {
this.logger.info('[ProxyCacheService:getProxyStreamResponse] %s, status: %s', url, res.status);
return res;
}

private async getUpstreamFullManifests(fullname: string): Promise<HttpClientResponse> {
const url = `/${encodeURIComponent(fullname)}?t=${Date.now()}&cache=0`;
return await this.getProxyResponse({ url }, { dataType: 'json' });
}

private async getUpstreamAbbreviatedManifests(fullname: string): Promise<HttpClientResponse> {
const url = `/${encodeURIComponent(fullname)}?t=${Date.now()}&cache=0`;
return await this.getProxyResponse({ url, headers: { accept: ABBREVIATED_META_TYPE } }, { dataType: 'json' });
}
private async getUpstreamPackageVersionManifest(fullname: string, versionOrTag: string): Promise<HttpClientResponse> {
const url = `/${encodeURIComponent(fullname)}/${encodeURIComponent(versionOrTag)}`;
return await this.getProxyResponse({ url }, { dataType: 'json' });
}
private async getUpstreamAbbreviatedPackageVersionManifest(fullname: string, versionOrTag: string): Promise<HttpClientResponse> {
const url = `/${encodeURIComponent(fullname)}/${encodeURIComponent(versionOrTag)}`;
return await this.getProxyResponse({ url, headers: { accept: ABBREVIATED_META_TYPE } }, { dataType: 'json' });
}

}
39 changes: 13 additions & 26 deletions app/port/controller/ProxyCacheController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
Context,
EggContext,
} from '@eggjs/tegg';
import { ForbiddenError, NotFoundError, UnauthorizedError } from 'egg-errors';
import { ForbiddenError, NotFoundError, UnauthorizedError, NotImplementedError } from 'egg-errors';
import { AbstractController } from './AbstractController';
import { ProxyCacheRepository } from '../../repository/ProxyCacheRepository';
import { Static } from 'egg-typebox-validate/typebox';
Expand All @@ -18,8 +18,8 @@ import {
ProxyCacheService,
isPkgManifest,
} from '../../core/service/ProxyCacheService';
import { SyncMode, PROXY_CACHE_DIR_NAME } from '../../common/constants';
import { NFSAdapter } from '../../common/adapter/NFSAdapter';
import { SyncMode } from '../../common/constants';
import { CacheService } from '../../core/service/CacheService';

@HTTPController()
export class ProxyCacheController extends AbstractController {
Expand All @@ -28,7 +28,7 @@ export class ProxyCacheController extends AbstractController {
@Inject()
private readonly proxyCacheService: ProxyCacheService;
@Inject()
private readonly nfsAdapter: NFSAdapter;
private readonly cacheService: CacheService;

@HTTPMethod({
method: HTTPMethodEnum.GET,
Expand Down Expand Up @@ -77,11 +77,12 @@ export class ProxyCacheController extends AbstractController {
if (refreshList.length === 0) {
throw new NotFoundError();
}
await this.cacheService.removeCache(fullname);
const taskList = refreshList
// 仅manifests需要更新，指定版本的package.json文件发布后不会改变
// only refresh package.json and abbreviated.json
.filter(i => isPkgManifest(i.fileType))
.map(async item => {
const task = await this.proxyCacheService.createTask(
.map(item => {
const task = this.proxyCacheService.createTask(
`${item.fullname}/${item.fileType}`,
{
fullname: item.fullname,
Expand All @@ -90,22 +91,18 @@ export class ProxyCacheController extends AbstractController {
);
return task;
});
const tasks = await Promise.all(taskList);
return {
ok: true,
tasks: await Promise.all(taskList),
tasks,
};
}

@HTTPMethod({
method: HTTPMethodEnum.DELETE,
path: `/-/proxy-cache/:fullname(${FULLNAME_REG_STRING})`,
})
async removeProxyCaches(@Context() ctx: EggContext, @HTTPParam() fullname: string) {
const isAdmin = await this.userRoleManager.isAdmin(ctx);
if (!isAdmin) {
throw new UnauthorizedError('only admin can do this');
}

async removeProxyCaches(@HTTPParam() fullname: string) {
if (this.config.cnpmcore.syncMode !== SyncMode.proxy) {
throw new ForbiddenError('proxy mode is not enabled');
}
Expand All @@ -116,6 +113,7 @@ export class ProxyCacheController extends AbstractController {
if (proxyCachesList.length === 0) {
throw new NotFoundError();
}
await this.cacheService.removeCache(fullname);
const removingList = proxyCachesList.map(item => {
return this.proxyCacheService.removeProxyCache(item.fullname, item.fileType, item.version);
});
Expand All @@ -140,17 +138,6 @@ export class ProxyCacheController extends AbstractController {
throw new ForbiddenError('proxy mode is not enabled');
}

await this.proxyCacheRepository.truncateProxyCache();
// 尝试删除proxy cache目录，若失败可手动管理
ctx.runInBackground(async () => {
try {
await this.nfsAdapter.remove(`/${PROXY_CACHE_DIR_NAME}`);
} catch (err) {
this.logger.error('[ProxyCacheService.truncateProxyCaches] remove proxy cache dir error: %s', err);
}
});
return {
ok: true,
};
throw new NotImplementedError('not implemented yet');
}
}
3 changes: 2 additions & 1 deletion app/port/controller/package/ShowPackageController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ export class ShowPackageController extends AbstractController {
if (this.config.cnpmcore.syncMode === SyncMode.proxy) {
// proxy mode
const fileType = isFullManifests ? DIST_NAMES.FULL_MANIFESTS : DIST_NAMES.ABBREVIATED_MANIFESTS;
const pkgManifest = await this.proxyCacheService.getPackageManifest(fullname, fileType);
const { data: sourceManifest } = await this.proxyCacheService.getProxyResponse(ctx, { dataType: 'json' });
const pkgManifest = this.proxyCacheService.replaceTarballUrl(sourceManifest, fileType);

const nfsBytes = Buffer.from(JSON.stringify(pkgManifest));
const { shasum: etag } = await calculateIntegrity(nfsBytes);
Expand Down
Loading

0 comments on commit 2780c53

Please sign in to comment.